Übungsprogramm "Kinokarten"

[Fritzew]

Hallo
nahpets hat ja schon einiges genannt,

wenn Du jetzt noch "alle" Berechnungen in Deine Kinoklasse bringst, wird es schon besser.
Also diese Zeilen:

Delphi-Quellcode:

       Kinokarten.ZwSu:=Kinokarten.PreisR[Rang1] + Kinokarten.PreisR[Rang2] + Kinokarten.PreisR[Rang3];
        Kinokarten.MWSt:=Kinokarten.ZwSu * MWSt;
        Kinokarten.Preis:=Kinokarten.ZwSu+Kinokarten.MWSt;

Berechnungen gehören in die Klasse und nicht in das Formular

[EdAdvokat]

Erst einmal vielen Dank, werde ich genau prüfen. Wie sieht es jedoch hinsichtlich der Deklaration public und private aus. Da ich Schwierigkeiten mit dem Zugriff hatte habe ich alles public erklärt und das ist wohl nicht richtig. Was wäre da zu tun?

[EdAdvokat]

Vielen Dank für die Hinweise. Ich hoffe, sie alle richtig umgesetzt zu haben. Bei der Prüfung konnte ich keine Fehler feststellen, doch sicher ist die von mir gewählte Deklaration public nicht so der Hit.
Verdient das Programm somit die Bezeichnung kleines Übungsprogramm für ein "Dorfkino"? Ich habe jedenfalls in der Erstellungsphase viel dazugelernt. Nochmals Dank

[Fritzew]

Hallo

Zitat von EdAdvokat:

Vielen Dank für die Hinweise. Ich hoffe, sie alle richtig umgesetzt zu haben. Bei der Prüfung konnte ich keine Fehler feststellen, doch sicher ist die von mir gewählte Deklaration public nicht so der Hit.
Verdient das Programm somit die Bezeichnung kleines Übungsprogramm für ein "Dorfkino"? Ich habe jedenfalls in der Erstellungsphase viel dazugelernt. Nochmals Dank

Also ganz ehrlich? Nein

Du hast immer noch zuviel Logik im Formular.

z.B:

Delphi-Quellcode:

Kinokarten.setAnzK1(strtoint(EAnzK1.text));
      if Kinokarten.Karten[Rang1] > (ord(frPlR1Vorg)) then
        begin
            ShowMessage(zuvielKarten);
            EAnzK1.Text:='0';
            EAnzK1.SetFocus;
            exit;
        end;

da gehört die Prüfung in die Klasse (meiner Meinung nach).

etwa in dem Kinokarten.setAnzK1 als function deklariert wird mit einem Boolean
also etwa

Delphi-Quellcode:

function TKinokarten.setAnzK1(k1:integer) : boolean;
begin
 result := false;
 if gekaufteKarten[Rang1] + k1 <= (ord(frPlR1Vorg)) then
  begin
    Karten[Rang1]:= k1;
    result := true;
  end;
end;

// in Auswerten dann:
 if not Kinokarten.setAnzK1(strtoint(EAnzK1.text)) then
        begin
            ShowMessage(zuvielKarten);
            EAnzK1.Text:='0';
            EAnzK1.SetFocus;
            exit;
        end;

Im Moment kann ich 2 * 12 Karten Rang1 kaufen bevor eine Meldung kommt........

[EdAdvokat]

Hallo zusammen, hier nun das Programm mit zwei units und den angemahnten Verbesserungen. Habe all die Anregungen hoffentlich ordentlich verarbeitet. Nach meiner Prüfung sollte es nun funktionieren. (12x12 verflickst, klappt nun nicht mehr!) Nochmals vielen Dank für die Anregungen, auf die ich z.T selbst wohl nicht gekommen wäre, doch es ist einfach Klasse diese Lösung, die man sich merken sollte. Das Design ist sicher nur "Dorfkino"-gerecht, doch daauf kommt es bei einem Übungsprogramm sicher nicht an, denn die programmtechnische Umsetzung sollte im Vordergrund stehen. Ist es nun so, dass man es ohne schlechtes Gewissen anbieten kann? (Ich will es jedoch nirgendwo anders als hier in der DP zeigen!)

[nahpets]

Im FormCreate fehlt noch das Form.Init.

In der Kinoklasse ist alles Public. Das finde ich nicht gut.

Also bitte überlegen, welche Funktionen der Klasse von außen zugreifbar sein müssen, welche nur intern zu verwenden sind und das entsprechend "umverteilen".

Eher unprofessionelles Vorgehen:
Alles Privat machen und nur die vom Compiler bemängelten Sachen wieder nach Public verschieben, der Rest muss nicht von außen zu sehen sein.

Danach überlegen, was man vom Prozeduren bzw. Funktionen zu Eigenschaften (mit Getter und Setter) umzuwandeln könnte.

Da die meisten Meldungen bereits über Konstanten geregelt sind, die hier bitte auch noch:

showMessage('alle '+inttostr(PlGes)+' Karten verkauft!'); Allerdings: Ist es überhaupt sinnvoll, dass die Kinoklasse 'ne Meldung ausgibt?

Sollte man nicht lieber aus der Prozedure, die die Meldung ausgibt, eine Funktion machen und an der Stelle des Progammes, an der sie aufgerufen wird, den Rückgabewert abfragen und dort dann entsprechend reagieren?

Gehen wir davon aus, jemand möchte die Klasse nutzen, aber in einer anderen Sprache. Er muss so auch die Klasse ändern. Bei 'nem auswertbaren Rückgabewert kann er aber selbst mit 'ner eigenen Meldung reagieren.

Sprich: Muss nur den Inhalt der Konstanten im "Hauptprogramm" ändern und gut is.

Fortgeschritten: ResourceStrings verwenden.

Oder die Texte in 'ner INI-Datei ablegen und im Formular.Init einlesen. Dann kann man die Meldungen auch ändern, ohne das Programm neu kompilieren zu müssen.

Bei den Meldungen showMessage(format(Verkaufsmeldung,[20,1,15,14])); stehen rechts vier Zahlen, die man irgendwo auch in der Kinoklasse wiederfindet. Wie wäre es, wenn die Klasse ein paar Attribute bekommt, die diese Werte "liefern"?

Momentan muss man bei Änderungen an zwei Stellen: Die Meldungen und die Klasse.

Genug für's Erste, weitere Ideen eventuell später

[Fritzew]

Schon besser,

schau Dir mal das hier im Anhang an. Habe es mal etwas umgebaut.
War nur auf die Schnelle aber vielleicht gibt es Dir ein paar Ideen.
Ach ja MwSt für Privatkunden inclusive. Sollte da nicht umgekehrt gerechnet werden?

[ub60]

Hallo,

ich habe mir mal die erste und die letzte Version Deines Programms heruntergeladen und möchte Dich beglückwünschen, ob der vielen Fortschritte, die Du gemacht hast.
Leider ist das Programm noch weit von "sehr gut" entfernt, so dass ich daran noch etwas basteln würde:

• Wie gerade schon geschrieben wurde, gehören alle Variablen (Attribute) der Klasse in protected (eventuell sogar in private).
• Auch wenn es ein Übungsprogramm ist, würde ich unbedingt das Layout ändern. Alle Worte sollten ausgeschrieben werden und inhaltlich zusammengehörende Felder könnte man z.B. in einer GroupBox mit einer passenden Überschrift zusammenfassen.
• Gerade wenn Du das Programm weitergeben möchtest, solltest Du noch etwas an den Variablen-, Komponenten- und Methodennamen feilen (AfrPlR3, getgekK2, ). Längere Namen tun wirklich nicht weh .
• Warum rufst Du zur Ermittlung der noch vorhandenen Karten nicht einfach die entsprechenden Get-Methoden auf? Dafür sind diese Methoden da. Dann könntest Du die Set-Methoden auch als Prozeduren laufen lassen und nicht als Funktionen (das ist eigentlich bei einfachen Set-Methoden üblicher.)
• Noch eine Kleinigkeit zum Quelltext-Layout: Einrückungen sollten generell 2 Zeichen und nicht abwechselnd 2, 3 oder 4 Zeichen sein.
• Beim Aufruf der folgenden Art ist die erste Zeile überflüssig. Sie wird nicht ausgewertet und erfolgt in der zweiten Zeile nochmals.

Code:

   Kinokarten.setAnzK1ber(strtoint(EAnzK1.text));
      if not Kinokarten.setAnzK1ber(strtoint(EAnzK1.text)) then

ub60

[EdAdvokat]

Hallo, leider habe ich eine falsche Version hochgeladen. Hier nun die hoffentlich richtige

[EdAdvokat]

Hallo zusammen, vielen Dank für die rege Kritik. Das muss ich erst noch ganz in Ruhe verarbeiten. Besonderen Dank an Fritzew für den Vorschlag, den ich noch näher beschauen muss. Doch das ist dann ganz sicher nicht mehr eine Programmierung die ich als weitgehend meine erklären könnte. Auch vielen Dan an nahpes und ub60 für die Hinweise, die ich als Problem z.T. selbst auch erkannt hat. Besonders das Problem public, private ... Hier habe ich einiges ausprobiert, jedoch in den jetzigen Varianten bleibt mir nichts anders übrig als alles public zu erklären. Das geht natürlich gar nicht, denn Variablen sollte man beispielsweise schützen. Hier müßte ich wohl völlig umdenken.
Es ist in der Tat so, dass ich durch dieses Projekt gemeinsam mit Euch unwahrscheinlich viel praktisch gelernt habe, denn ich möchte mich als eher Anfänger einschätzen. Die jetzigen Vorschläge von Euch fordern mich an der oberen Grenze und ich muss dazu sicher noch mal in meine schlauen Bücher sehen.