einfache Texterkennung - SPOJ Hotline Problem

[DasIch]

Ich bin gerade dabei das Hotline Problem von SPOJ zu lösen. Die Erkennung des Inputs hab ich jetzt zumindest hinbekommen
Das dass das erste ist was ich (versucht hab) Objekt orientiert zu programmieren würde mich interessieren was ich da noch verbessern könnte.
http://paste.pocoo.org/show/56034/

[BlackJack]

WTF!? Du könntest es neu schreiben. Das ist alles sehr verwirrend. Du greifst auf globale Objekte zu, die den gleichen Namen wie Klassen haben, aber erst nach diesen Klassen und nach den Methoden die darauf zu greifen, definiert werden. So etwas habe ich bisher noch nie gesehen!

[audax]

Mal als Anregung:

Code: Alles auswählen

        if General.sentence_type == 'statement':
            self.subject = self.person_types[General.word_list[0]]
            return self.subject
        if General.question_type == 'do|does':
            self.subject = self.person_types[General.word_list[1]]
            return self.subject
        elif General.question_type == 'anything':
            self.subject = self.person_types[General.word_list[2]]
            return self.subject

            # und hier meine Variante
        self.subject = self.person_types[General.word_list[
            { 'statement' : 0,
            'do|does'   : 1,
            'anything'  : 2,
            }[General.sentence_type]
        return self.subject

        # außerdem:
        ('''don't''','''doesn't''') == ("don't", "doesn't")

So allgemein ist dein Code allerdings höchst verwirrend. Du erstellst eine Klasse und baust später davon ein Exemplar mit identischem Namen Oo

Außerdem verstehe ich den Sinn hier nicht:

Code: Alles auswählen

    def __init__(self, General):
        self.person_types = {'i':'1ps','you':'2ps','everybody':'3ps','nobody':'3ps'}

Du lässt dir `General` übergeben, tust aber nichts?
Und warum definierst du person_types erst beim erstellen des Exemplars und nicht schon vorher?

Also eigentlich ist es bei OOP so, dass Objekte miteinander kommunizieren, und zwar nicht über globale Variablen

[DasIch]

Ich hab das jetzt mal nochmal neu geschrieben. Wäre das in Ordnung?
http://paste.pocoo.org/show/57728/

[audax]

Ich versteh den Code ehrlich gesagt nicht.

Code: Alles auswählen

sentence.sentence.startswith(word)

Auch so ein Juwel

Also...schreib den Code mal mit Kommentaren, in denen du Sinn und Zweck der einzelnen Klassen, Methoden und Attribute erklärst.

Code: Alles auswählen

		try:
			return sentence.split()
		except:
			return self.sentence.split()

Und das ist rein handwerklich blöd. Niemals ein reines except nehmen...

Mehr kann ich zu dem Code nicht sagen, da ich nicht durchblicke.
Mach ne ordentliche Doku bis pylint nicht mehr meckert und auch andere Menschen den Code lesen können

[BlackJack]

Ich denke zwanghaft die Sätze in Objekte zu giessen ist nicht der "direkteste" Weg zur Lösung. Denn was man wirklich modellieren muss, ist eine Datenstruktur, in der die Fakten möglichst so gespeichert werden, dass man die Fragen effizient beantworten kann. Das zerlegen der Eingabe kann man auch mit Funktionen erledigen.

Zumindest von den Namen her ist `Words` auch ein wenig fragwürdig. Bei Vererbung sollte in der Regel eine "ist ein(e)"-Beziehung zwischen Klassen gelten. Und Worte sind in dem Sinne keine Frage oder ein Satz, sondern ein Satz oder eine Frage *bestehen* aus Worten. Das deutet eher auf Komposition als auf Vererbung hin.

Rein technisch stellt sich auch die Frage warum `Words` von `Sentence` erbt, wo das doch über `Question` schon passiert ist.

[DasIch]

Rein technisch stellt sich auch die Frage warum `Words` von `Sentence` erbt, wo das doch über `Question` schon passiert ist.

Ich war mir nicht sicher ob das so einwandfrei geht. Werde es aber ändern.

Die Klasse Words zu nennen ist nicht sonderlich geschickt das weiß ich auch, mir ist allerdings nicht wirklich eingefallen wie ich das möglichst korrekt und kurz benennen soll. Ich hab das ganze prinzipiell schonmal mit Funktionen gelöst aber ich muss sagen wenn man es einmal verstanden hat gefällt mir die Objekt orientierte Variante besser und es erscheint mir auch sauberere zu warten. Naja um prinzipiell mal ein paar Sachen zu erklären:
Sentence stellt als Klasse erstmal den Eingegebenen Satz dar und davon lässt sich eine Liste mir Wörter(word_list()) bekommen, den Typ des Satzes, also einfaches Aussagesatz oder Frage und ob der Satz verneint ist.

Question ist eigentlich nur für den Fall da, dass das ganze eine Frage ist und ich den Typ davon brauche. Zugegeben ich hätte das vielleicht einfach in Sentence einbauen können und ich weiß momentan auch nicht mehr so genau wieso ich das überhaupt gemacht hab. Schliesslich hab ich auch keine Klasse statement was eigentlich dann logisch erscheint.

Words als Begriff ist wie vorher erwähnt für die letzte Klasse etwas ungenau. Eigentlich geht es darum die grammatikalisch wichtigen Wörter Subjekt, Prädikat und Objekt herauszubekommen.

Ich werd mir pylint mal ansehen(kannte ich bisher nicht) und mal versuchen sinnvolle(ere) Namen für die ganzen Sachen zu finden und natürlich Informationen was, wo, wie erreicht werden soll
Ehrlich gesagt bin ich ja schon froh dass der Code jetzt nicht völlig in der Luft zerissen wurde

EDIT:Hab mal das etwas seltsame sentence.sentence geändert und einige andere Sachen die pylint angestrichen hat behoben. Außerdem werden jetzt Leerzeichen statt Tabs zu Einrückung verwendet. Docstrings sowie ein paar Kommentare sind hinzugekommen. http://paste.pocoo.org/show/58278/

[BlackJack]

Sowohl von den Namen her, als auch von Deiner Beschreibung steht `Words` IMHO genau am falschen Ende der Vererbungshierarchie. Denn Subjekt, Verb, und Objekt haben sowohl Aussagen als auch Fragen. Das ist also eine Gemeinsamkeit, die man eventuell in einer Oberklasse modellieren kann. Falls der Code dafür der gleiche ist. Falls nicht, wär's eher eine abstrakte Oberklasse, die man sich in Python sparen könnte.

"Wenn man es erst einmal verstanden hat" ist das Problem was ich damit habe. IMHO ist diese Objekthierarchie komplexer als das mit einfachen Funktionen zu lösen. Und komplexerer Code ist nicht leichter zu verstehen und warten als einfacherer Code.

Bei der Aufgabenstellung sollte man sich IMHO eher Gedanken um eine Datenstruktur machen, in der man die Fakten speichert und die zu jedem Fragetyp eine effizient berechenbare Antwort erlaubt. Diese Datenstruktur ist das Herz des Programms und die "Satzobjekte" existieren alle nur ganz kurz.

Ansonsten: `Sentence.word_list()`: Die Überprüfung würde ich, wenn sie überhaupt nötig ist, umdrehen. Nicht für jedes Satzzeichen testen ob's am Ende von der Zeichenkette steht, sondern ob das Ende der Zeichenkette ein Satzzeichen ist. Das wäre ein Test mit ``in`` und man bräuchte keine Schleife. Allerdings habe ich die Aufgabenstellung so verstanden, dass jeder Satz mit einem der Zeichen '.?!' endet, also sollte man grundsätzlich das letzte Zeichen vor dem Zerlegen entfernen können.

`Sentence.is_negated()` ist fehlerhaft. Die Sätze "Peter likes nobody." oder "Metallica plays "don't tread on me".' sind zum Beispiel nicht "negated", werden von der Funktion aber als solche erkannt. (Wobei ersteres glaube ich von den Regeln ausgeschlossen wird.)

In `Question` und `Words` wird die `__init__()` der Oberklasse nicht aufgerufen. `Question.type()` überschreibt `Sentence.type()` mit einer völlig anderen Bedeutung. Böse Fehler.

`Questions.type()` ist äusserst umständlich geschrieben. Dictionaries bieten schnellen Zugriff auf Werte über Schlüssel und Du schreibst dafür eine Schleife die jeden einzelnen Schlüssel überprüft. Und wo zum Henker kommt `sentence` in dieser Funktion her? Das ist wieder kein OOP wenn die Objekte über globale Namen kommunizieren. Steck doch mal den Code auf Modulebene in eine `main()`-Funktion. Auf alles auf dass Du dann plötzlich nicht mehr einfach so zu greifen kannst *solltest* Du auch nicht zu greifen können!

Die ganzen Fallunterscheidungen in `Words` bekräftigen meine Vermutung, dass diese Klasse an der falschen Stelle in der Vererbungshierarchie steht. Sowohl Aussagen als auch Fragen haben Subjekt, Prädikat, und Objekt. Der Unterschied liegt (eventuell) nur daran, wo die im Satz stehen. Also sollte die Verantwortlichkeit für das Ermitteln auch in den entsprechenden konkreten Klassen gekapselt sein. Diese ``if``-Kaskaden sind schlecht erweiterbare prozedurale Programmierung, die man mit OOP, Stichwort Polymorphie, gerne loswerden möchte.

Alles in allem ist das kein OOP, sondern nur Funktion, die über globale Namen Daten austauschen in Klassen gesteckt. Die Klassen verschlechtern das Programm nur.