Wie ist der Programmierstil "professionell"

[coderi]

Hallo Zusammen,

ich habe hier mal eine Grundsatzfrage zum Thema Programmieren in Delphi

Delphi-Quellcode:

procedure TfrmXXX.ZeileDelExecute(Sender: TObject);
begin

   dmXXX.qGETFertigteileeingang.ParamByName('AZID').AsInteger := dmXXX.AZQ.fieldbyname('AZ.ID').asinteger;
   dmXXX.qGETFertigteileeingang.Open;
   if (dmXXX.qGETFertigteileeingang.FieldByName('Anzahl').AsInteger > 0) then
   begin
      MessageDlg('Löschen der Auftragszeile NICHT möglich, da bereits Fertigteile daraus eingebucht wurden!', mtError, [mbOK], 0);
      dmXXX.qGETFertigteileeingang.close;
      exit;
   end;
   dmXXX.qGETFertigteileeingang.close;
    .
. .
.. 
code hier noch nicht zu ende.

Erklärung:

Es geht hier um einen Rowcount mit anschließender Auswertung, wenn die Anzahl > 0 dann soll er nach Meldung direkt abbrechen und die Procedure verlassen.
Die query möchte ich aber immer geschlossen wissen.

Frage:
Unter Berücksichtigung von gutem Programmierstyle, Codewartbarkeit u.a. :

Ist es so korrekt wie oben dargestellt?

Oder benutze ich eine Variable als Zwischenspeicher wie hier:

Delphi-Quellcode:

procedure TfrmAuftrag.ZeileDelExecute(Sender: TObject);
var
 azid:Integer;
begin

   dmAuftrag.qGETFertigteileeingang.ParamByName('AZID').AsInteger := dmauftrag.AZQ.fieldbyname('AZ.ID').asinteger;
   dmAuftrag.qGETFertigteileeingang.Open;
   azid := dmAuftrag.qGETFertigteileeingang.FieldByName('Anzahl').AsInteger;
   dmAuftrag.qGETFertigteileeingang.close;
   if (azid > 0) then
   begin
      MessageDlg('Löschen der Auftragszeile NICHT möglich, da bereits Fertigteile daraus eingebucht wurden!', mtError, [mbOK], 0);
      exit;
   end;

Oder gib es einen ganz anderen Ansatz dafür?

Antworte bitte mit Begründung :-)

danke euch
grüße
Coderi

[Bjoerk]

Die Extravariable ist wohl eher Geschmackssache, aber Exit verwende ich persönlich nur nur, wenn es gar nicht anders geht, dafür aber öfter mal ein try .. finally.

Delphi-Quellcode:

   dmXXX.qGETFertigteileeingang.ParamByName('AZID').AsInteger := dmXXX.AZQ.fieldbyname('AZ.ID').asinteger;
   dmXXX.qGETFertigteileeingang.Open;
   try
     if (dmXXX.qGETFertigteileeingang.FieldByName('Anzahl').AsInteger > 0) then
       MessageDlg(' .. ', mtError, [mbOK], 0)
     else
     begin

     end;
   finally
     dmXXX.qGETFertigteileeingang.close;
   end;

[Stevie]

Ob nun Variable, Exit oder nicht - mir fällt da was ganz anderes auf in Hinsicht auf Wartbarkeit. Es handelt um etwas, was mit dem Dataset gemacht wird. Dennoch ist diese Logik scheinbar in einem Form oder Frame implementiert - warum nicht im Datamodule?

In der jetzigen Form kannst du das Datamodule nicht separat einsetzen bzw testen, es funktioniert nur in Verbindung mit diesem Form/Frame.

[p80286]

Um Stevies Bemerkung aufzunehmen, besser wäre wohl so etwas:

Delphi-Quellcode:

------
Datamodule:
function Isrecordused(recnr:integer):boolean;
....
result:=dmAuftrag.qGETFertigteileeingang.FieldByName('Anzahl').AsInteger>0;
...
end;

function Delete(recnr);
....
end;
-----------------
Hauptprogramm:
if not(IsrecordUsed(recnr) then delete(recnr);

Wenn du dich an dieses Prinzip hälst kannst Du Problemlos die DB-Schnittstelle austauschen, ohne am eigentlichen Programm eine Änderung durchzuführen.
(Namen sind Schall und Rauch! das Delete ist nicht der Weisheit letzter Schluß!)

Gruß
K-H

[Sir Rufo]

Zitat:

Zitat von Bjoerk (Beitrag 1191560)

Die Extravariable ist wohl eher Geschmackssache, aber Exit verwende ich persönlich nur nur, wenn es gar nicht anders geht, dafür aber öfter mal ein try .. finally.

Bei deinem Konstrukt kann man getrost Exit verwenden, weil der finally Block immer ausgeführt wird.

Delphi-Quellcode:

procedure SomeStuff;
begin
  SomeData.Open;
  try // Wenn man hier drüber weg ist, dann ...
    ...
    if SomeThing then
      Exit;
    ...
  finally // ... wird dieser Block immer ausgeführt
    SomeData.Close;
  end;
end;

[sx2008]

Wenn's "professionell" sein soll, dann muss man die Businesslogik extrahieren und entsprechend abbilden.
Wenn man viele Anweisung hat, die mehr als einen Punkt enthalten, dann müssen die Alarmglocken klingeln:

Delphi-Quellcode:

procedure TfrmXXX.ZeileDelExecute(Sender: TObject);
begin
   // wir befinden uns in der Klasse TfrmXXX
   // und trotzdem greifen wir immer wieder auf die internen Dinge des Datenmoduls
   // TdmXXX zu
   dmXXX.qGETFertigteileeingang.ParamByName('AZID').AsInteger := dmXXX.AZQ.fieldbyname('AZ.ID').asinteger;
   // und hier schon wieder - 2. Punkte in einer Anweisung
   // das "stinkt"
   dmXXX.qGETFertigteileeingang.Open;

Also es ist klar, der Code zum Auslesen der Anzahl der Fertigteile gehört in das Datenmodul

Delphi-Quellcode:

function TdmXXX.GetFertigteileAnzahl(azid : Integer):integer;
begin
   qGETFertigteileeingang.ParamByName('AZID').AsInteger := azid;
   qGETFertigteileeingang.Open;
   result := qGETFertigteileeingang.FieldByName('Anzahl').AsInteger;
   qGETFertigteileeingang.close;
end;

Und jetzt zum 2. Problem:
Es wird MessageDlg() verwendet wo eigentlich eine Exception ausgelöst werden sollte:

Delphi-Quellcode:

procedure TfrmXXX.ZeileDelExecute(Sender: TObject);
var
  anzahl : Integer;
  azid : Integer;
begin
  azid := dmXXX.AZQ.fieldbyname('AZ.ID').asinteger;
  anzahl := dmXXX.GetFertigteileAnzahl(azid);
  if (anzahl > 0) then
    // Exception auslösen und dem Benutzer möglichst genaue Info geben wo das Problem ist
    raise Exception.CreateFmt('Löschen der Auftragszeile NICHT möglich, da bereits %d Fertigteile daraus eingebucht wurden (azid=%d)', [anzahl, azid]);

[Stevie]

Zitat:

Zitat von sx2008 (Beitrag 1191632)

Und jetzt zum 2. Problem:
Es wird MessageDlg() verwendet wo eigentlich eine Exception ausgelöst werden sollte:

Delphi-Quellcode:

procedure TfrmXXX.ZeileDelExecute(Sender: TObject);
var
  anzahl : Integer;
  azid : Integer;
begin
  azid := dmXXX.AZQ.fieldbyname('AZ.ID').asinteger;
  anzahl := dmXXX.GetFertigteileAnzahl(azid);
  if (anzahl > 0) then
    // Exception auslösen und dem Benutzer möglichst genaue Info geben wo das Problem ist
    raise Exception.CreateFmt('Löschen der Auftragszeile NICHT möglich, da bereits %d Fertigteile daraus eingebucht wurden (azid=%d)', [anzahl, azid]);

Und selbst diese Methode gehört ins DM. Dann merkt man nämlich auch, worin der Vorteil der Exception gegenüber dem MessageDlg liegt: man hat keine GUI im DM. Wobei man sich darüber streiten kann, ob hier nicht lieber ein Result vom Typ Boolean einer Exception vorzuziehen wäre (Stichwort: keine Exceptions als Teil der Business Logic).

[Sir Rufo]

Und um nicht in Versuchung zu kommen, kann man dem DataModule auch ein Interface verpassen und der Form nur dieses Interface übergeben. Dann ist Schluss mit dem wilden Zugreifen auf die Komponenten ;)

[sx2008]

Zitat:

Zitat von Sir Rufo (Beitrag 1191688)

...kann man dem DataModule auch ein Interface verpassen

Aber man würde auch einen Preis dafür bezahlen.
1.) mehr Sourcecode
2.) falls man eine Methode ändern möchte, muss man an mindestens 3 Stellen ändern
im Interface, in der Datenmodul-Klasse und beim Aufrufer

Durch den zusätzlichen Aufwand auch noch das Interface zu ändern, werden einige Programmierer notwendige Änderungen unterlassen und sich irgendwie durchmogeln.
Vielleicht wird dann zum Teil über das Interface und daneben auch direkt auf das Datenmodul zugegriffen (was natürlich sehr schlecht wäre).

Interfaces haben eben ihre Vor- und Nachteile.

[Lemmy]

Zitat:

Zitat von coderi (Beitrag 1191557)

Frage:
Unter Berücksichtigung von gutem Programmierstyle, Codewartbarkeit u.a. :

Ist es so korrekt wie oben dargestellt?

Oder benutze ich eine Variable als Zwischenspeicher wie hier:

Steve McConnell beschreibt die Version mit der Variable als Zwischenspeicher als "geschwätzigen Code". Es besteht zudem die Gefahr, dass bei einer späteren Erweiterung des Codes die Versuchung nahe liegt die Variable für was anderes zu verwenden.

Es KÖNNTE jedoch hilfreich sein, eine Variable mit einem starken Namen einzuführen, damit diese den Code besser dokumentiert als so ein Monsterausdruck.


Alles in allem also eher eine Frage des Geschmacks, der Situation und der Begleitumstände.

[Bentissimo]

Hallo zusammen,

imho kann die Funktionalität am besten wie folgt umgesetzt werden:

1. Wie bereits mehrfach erwähnt, würde ich auf alle Fälle eine Methode eines DataModule dafür verwenden.
2. Eine mögliche Funktion sollte weder eine MessageBox ausgeben noch eine Exception auslösen, sondern als Rückgabewert einen Fehlercode liefern. Wie dieser dann ausgewertet und gegebenenfalls angezeigt wird, entscheidet der aufrufende Programmteil.
3. Statt das Dataset zu öffnen würde ich dynamisch eine Query erzeugen, ausführen, Ergebnis merken, schliessen und freigeben. Zur Ermittlung des Ergebnisses darf dann gerne ein try... except eingesetzt werden.

Ein Beispiel dafür könnte wie folgt aussehen:

Delphi-Quellcode:

const
  sqlCheckDeletable =
    'SELECT ...' + #13#10 +
    '  FROM ...' + #13#10 +
    ' WHERE ... = :AID';

  ecNotDeletable = 1;

function TMyDataModule.CheckDeletable(AID: Integer): Integer;
var
  Q: TQuery;
begin
  Result := 0;
  Q := TQuery.Create(nil);
  try
    Q.DataBaseName := '...';
    Q.SQL.Text := sqlCheckDeletable;
    Q.ParamByName('...').AsInteger := AID;
    try
      Q.Open;
      if Q.Fields[0].AsInteger > 0 then
        Result := ecNotDeletable;
      Q.Close;
    except
      on E: EDatabaseError do
        Result := E.ErrorCode;
    end;
  finally
    Q.Free;
  end;
end;

Viele Grüsse
Stephan

[coderi]

Ist zwar schon etwas her, aber das Forum vergisst nichts !

Vielen Dank für eure Ausführungen, ist stimme mit euch überein, das die Logik wohl eher in ein Datenmodul, oder
besser noch in eine zentrale Funktion gehört. Ich persönlich würde mich mittlerweile für den letzten tip von Stephan entscheiden,
da ich hier die "zentralste" und einfachste Möglichkeit sehe das ganze ohne viel primbamorium zu lösen, und auch zu verwalten.

Aber einen Fehlercode würde ich nicht zurückgeben, kann ich doch eine exeption "werfen", und die an beliebiger stelle "Fangen",
um ihn so, in einer mehr schichten welt, an der passenden stelle zu plazieren.
Ein weiterer Vorteil von exceptions:
Fehlercodes muss ich in einer liste vorhalten, einer Expetion gebe ich den, auch für den Benutzer aussagekräftigen Fehler,
gleich mit. Für den Fehlercode brauche ich eine extra variable.

grüße
Coderi

[Bentissimo]

Eine Exception auszulösen ist sicher in Ordnung. Für mich sind Exceptions und das Werfen solcher aber eigentlich genau das, was sie sind: Ausnahmen.

Die Erkenntnis, dass ein Datensatz nicht gelöscht werden darf ist aber für mich kein solcher Fall, sondern schlichtweg die Bestätigung einer Geschäftsregel.

Insofern sehe ich hier keine Notwendigkeit für eine Exception.

Nun mag mein Programmierstil altersbedingt schon etwas angestaubt sein. :roll:

Gibt es dazu weitere Meinungen? Würde mich interessieren.

Viele Grüsse.

[Lemmy]

Zitat:

Zitat von Bentissimo (Beitrag 1199581)

Eine Exception auszulösen ist sicher in Ordnung. Für mich sind Exceptions und das Werfen solcher aber eigentlich genau das, was sie sind: Ausnahmen.

Die Erkenntnis, dass ein Datensatz nicht gelöscht werden darf ist aber für mich kein solcher Fall, sondern schlichtweg die Bestätigung einer Geschäftsregel.

sehe ich genauso.

[stahli]

@Bentissimo

Ich sehe das ganz genau so (obwohl ich noch deutlich jünger bin ;-)).

Übrigens auch in Bezug auf try-finally-Blöcke (habe ich schon öfters erklärt). Es bringt m.E. nichts, ein Objekt aufzuräumen wenn das Projekt ohnehin in einem unkontrollierten Status ist.
Ich erzeuge ein Objekt und führe komplexe Handlungen aus. Irgendwo mittendrin knallt es. Macht nix, ich räume das Objekt weg und gut ist???
Der Fehler muss bereinigt bzw. umgangen werden. Sensible Daten müssen ggf. gesichert und das laufende Projekt muss ggf. neu gesatartet werden um einen kontrolierten Zustand zu erhalten.

Try-Except macht Sinn, wenn ich immer mit einem unkontrollierten Zustand rechnen muss (z.B. mei IO-Funktionen) und der Programmzustand auch nach der Fehlerbehandlung/Unterdrückung noch in Ordnung ist.

Objektfreigaben "zwanghaft" in einen Finally-Block aufzunehmen, halte ich für unnötig.


[Werbeblock]
Hast Du Dir mal meine Turniersoftware in Bezug auf Schach angesehen?
[/Werbeblock]

[Bentissimo]

@Stahli

Habe mir am vergangenen Wochenende die Testversion Deiner Turniersoftware runter geladen, daher vermutlich Deine Frage. :stupid:

Allerdings nicht in Sachen Schach, sondern mehr für ein kleines Backgammon-Turnier. 8-)

Bin derzeit in einem Projekt in der Schweiz und mein Scrum-Master kam irgendwie auf Backgammon. Ich war früher mal ganz gut und so haben wir in der Mittagspause ein bischen gespielt. Mittlerweile soll ich ein Turnier für sechs Leute organisieren und hab deshalb an Deine Software gedacht. :wink:

Bitte nicht böse sein, aber ich kam irgendwie nicht damit zurecht. :(

Die Idee war, schnell mal ein Round-Robbin anzulegen, sechs Namen einzutragen und dann per Knopfdruck die Runden und die Tabelle angezeigt zu bekommen.

So lief es leider nicht, aber auch diesmal kann es natürlich an mir liegen. Wie formuliert das Bummi immer so schön?

Das Problem liegt meist zwischen den Ohren! :lol:

[stahli]

[OT]
@Bentissimo
Backgammon kenne ich nicht, aber wenn es um Punkte erspielen geht, dann sollte das funktionieren. Die Personen musst Du in einem (temporären) Verein definieren.
Den Rest erklären Dir meine Anwälte heute Abend per pm. ;-)
[/OT]