Programm "Waren"

[EdAdvokat]

Ein kleines Übungsprogramm "Waren" zur Disskussion. Es soll lediglich eine einfache Berechnung von Preisen, Rabatt und Warenbestand
dargestellt werden.

[EdAdvokat]

Hallo zusammen, gibt es überhaupt nichts zu meckern an dem Programm? So perfekt kann das doch nicht sein.
Zwar ist zu merken, dass es vermutlich Interesse findet, doch keiner meldet sich zu Wort. Bislang konnte
ich aus den konstruktiven Kritiken immer lernen und das soll ja ein Leben lang so weiter gehen ("man lernt
sein Leben lang niemals aus...") Also was ist zu kritisieren?

[HolgerX]

Hmm..

OK, fangen wir an zu zerlegen

Delphi-Quellcode:

procedure TWaren.init;
begin
  FillChar(FWarenbestand,sizeof(FWarenbestand),0);
  FillChar(FWarenverkauf,sizeof(FWarenVerkauf),0);
  FillChar(FaktBestand,sizeof(FaktBestand),0);
  FillChar(FStPreis,sizeof(FStPreis),0);
  FillChar(FVerkPreis,sizeof(FVerkPreis),0);
  FillChar(FNettopreis,sizeof(FNettoPreis),0);
  FillChar(FMWSt,sizeof(FMWSt),0);
  FillChar(FErtrag,sizeof(FErtrag),0);
  FillChar(FAbMWSt,sizeof(FAbMWSt),0);
end;

Wieso benutzt Du FillChar um Integer oder Float Variablen zu initialisieren?
Sollte dies nicht besser nur für arrays ob Byte/Char verwendet werden?

Auch ist es besser lesbar, wenn Du die Variablen mit dem entsprechenden Wert initialisierts...

Delphi-Quellcode:

procedure TWaren.init;
begin
  FWarenbestand := 0;
  FWarenverkauf := 0;
  FaktBestand   := 0;
  FStPreis      := 0;
  FVerkPreis    := 0.0;
  FNettopreis   := 0.0;
  FMWSt         := 0.0;
  FErtrag       := 0.0;
  FAbMWSt       := 0.0;
end;

Delphi-Quellcode:

var
  Form1: TForm1;
  Waren: TWaren;

Und was mich immer wieder den Kopf schütteln läst: Unsinnige GLOBALE Variablen!

'Waren' wird nur 'innerhalb' der FormularClasse verwendet, wieso dort nicht als FWaren : TWaren unter 'Private' anlegen?
Dann würde an jeder Stelle in den Methoden der Form erkennbar sein, wo es herkommt ('FWaren' = Membervar).

Wieso hat die Classe TWaren kein eigenes Init, bei dem z.B. Warenbe); //const Warenbe
gesetzt wird?

und beim Form1.Init sollten die Werte aus der Classe Waren (nach dessen Init) genommen werden und nicht wieder eigene Konstanten.

Delphi-Quellcode:

procedure TForm1.init;
begin
  FWaren.Init;

  Warenbestand.Text := IntToStr(FWaren.Warenbestand);
  WarenVerkauf.Text:='0';
  aktBestand.Text   := IntToStr(FWaren.aktBestand);
  StPreis.Text      := FloatToStr(FWaren.StPreis);  //const StkPreis
  VerkaufsPreis.text:= FloatToStr(FWaren.Verkaufspreis);
  Nettopreis.Text   := FloatToStr(FWaren.Nettopreis);
  MWSt.Text         := FloatToStr(FWaren.MWSt);
  Ertrag.Text       := FloatToStr(FWaren.Ertrag);
  AbMWSt.Text       := FloatToStr(FWaren.AbMWSt);
  BruttoUmsatz.Text := FloatToStr(FWaren.BruttoUmsatz); // Fehlt

  checkbox1.Checked      :=false;
  checkboxRabatt.Checked :=false;
end;

Somit wird nur an einer Stelle Konstanten ver wendet und diese zur Ansicht gebracht.


So, hoffe ein paar Tips geben zu können..

Mal sehen, ob meine Infos jetzt von anderen zerpflückt werden

[jaenicke]

Das größte Problem ist, dass die Mehrwertsteuerberechnung falsch ist. Die 19% Mehrwertsteuer werden auf den Nettowert draufgerechnet. Das sind nicht 19% des Bruttowerts...

[p80286]

Da ich nur D7 habe muß ich mich auf den Sourcecode beschränken:

Delphi-Quellcode:

   begin
          if Waren.Warenverkauf>=10 then
            checkboxRabatt.Checked:=true;
         if checkboxRabatt.Checked then
            Waren.StPreis:=((Waren.StPreis)-1);
            checkboxRabatt.Checked:=false;
        MessageDlg(format(Preismeldung,[(Waren.Verkaufspreis),(Waren.Nettopreis),(Waren.MWSt)]),mtInformation,[mbOk],0);
        end;

das checkboxRabatt.Checked:=false; gehört auf den ersten Blick noch zu if checkboxRabatt.Checked then was nach genauerem hinschauen Blödsinn ist, aber warum unnötig verkomplizieren?
Warum diese "Checkboxrabatt"-Orgie?
Wenn Du mehr als 10Stk verkaufst gibt es Rabatt egal ob die Checkbox gecheckt ist oder nicht?
Wenn ein Rabatt gewährt werden soll, dann sollte dies die Checkbox ändern. Es könnte sein, daß die Rabattgewährung nicht möglich ist. Das weiß aber Waren daher sollte Waren den Status der Checkbox setzen.

Delphi-Quellcode:

        if (Waren.aktBestand)<0 then
         begin
           showMessage(mehrWaren);
           checkbox1.Checked:=true;exit;
         end;

Showmessage soll informieren, daß es nicht mehr genug Waren gibt. Warum überprüft die Oberfläche dies und nicht die Klasse Waren?
Und wer ist checkbox1 Gruß
K-H

[Luckie]

Ich glaube, du begehst die gleichen Fehler wieder, wie bei deinem anderen Programm.

Aber Hut ab für deinen Mut so was hier vor zu stellen. Aber nur so lernt man, aus Fehlern. Weiter so.