Opened by jan99 at 2015-03-27 08:06
User since
2010-08-29
373 Artikel
BenutzerIn
![[default_avatar]](/battie/theme/default/default_avatar.gif)
2010-08-29
373 Artikel
BenutzerIn
IIRC war die ursprüngliche Aufgabenstellung so, dass da OOP keinen Nutzen brachte und man nur ein traditionelles Package brauchte um ein paar Funktionen zu bündeln und den Scope einiger statischer Variablen zu sinnvoll zu begrenzen.
Nun hast Du EBL::ebl_reporting doch objektorientiert aufgezogen, so dass Du ein Logger-Objekt vom Typ EBL::ebl_reporting anlegen und weiterreichen musst. In Deinem Beispiel bringt das keinen Nutzen. Aber vielleicht hast Du ja doch einmal eine Anwendung, wo Du zwei unterschiedliche EBL::ebl_reporting-Logger-Objekte parallel brauchst - nur dann bringt der OO-Ansatz Vorteile. Ansonsten sind statische log(), shell() und write() Funktionen besser.
Aber wenn du schon unbedingt objektorientiert programmieren willst, dann mach es wenigstens richtig. Dein Code ist grausam.
Dein open() ist schlimmster Spaghetti-Code, weil Du da 2-3 unterschiedliche Funktionalitäten in eine Methode presst und zunächst ein paar Zeilen von der ersten Funktionalität implementierst, dann kommen ein paar Zeilen von einer anderen Funktionalität, dann springst Du wieder zurück zur ersten Funktionalität und zwischendurch widmest Du Deine lokalen Variablen um, so dass sie je nach Funktionalität ganz unterschiedliche Bedeutungen haben.
Mach da besser drei unterschiedliche Methoden draus
Und wenn Du die Funktionalitäten sauber getrennt hast, kannst du wahrscheinlich auch die Zahl der lokalen Zwischenvariablen deutlich reduzieren.
Eine andere Grausamkeit: Dass Du in write() und doublewrite() die Argumente nicht sauber übergibst. Besser:
Da sieht man beim Lesen des Codes direkt, was du an Argumenten erwartest.
Das macht später die Pflege des Codes deutlich einfacher.
Nun hast Du EBL::ebl_reporting doch objektorientiert aufgezogen, so dass Du ein Logger-Objekt vom Typ EBL::ebl_reporting anlegen und weiterreichen musst. In Deinem Beispiel bringt das keinen Nutzen. Aber vielleicht hast Du ja doch einmal eine Anwendung, wo Du zwei unterschiedliche EBL::ebl_reporting-Logger-Objekte parallel brauchst - nur dann bringt der OO-Ansatz Vorteile. Ansonsten sind statische log(), shell() und write() Funktionen besser.
Aber wenn du schon unbedingt objektorientiert programmieren willst, dann mach es wenigstens richtig. Dein Code ist grausam.
Dein open() ist schlimmster Spaghetti-Code, weil Du da 2-3 unterschiedliche Funktionalitäten in eine Methode presst und zunächst ein paar Zeilen von der ersten Funktionalität implementierst, dann kommen ein paar Zeilen von einer anderen Funktionalität, dann springst Du wieder zurück zur ersten Funktionalität und zwischendurch widmest Du Deine lokalen Variablen um, so dass sie je nach Funktionalität ganz unterschiedliche Bedeutungen haben.
Mach da besser drei unterschiedliche Methoden draus
- new([<filename>]), die als erstes Argument die Klasse EBL::ebl_reporting übergeben bekommt und ein neues Objekt von diesem Typ anlegt und zurückgibt. Der Filename des Logfiles als zweites Argument ist optional.
- change_file(<filename>), um bei einem bestehenden Objekt den Logfile zu schließen und den Namen des Logfiles zu ändern
- open() um den Logfile zu öffnen.
Und wenn Du die Funktionalitäten sauber getrennt hast, kannst du wahrscheinlich auch die Zahl der lokalen Zwischenvariablen deutlich reduzieren.
Eine andere Grausamkeit: Dass Du in write() und doublewrite() die Argumente nicht sauber übergibst. Besser:
Code (perl): (dl
)
1 2 3 4 5
sub write { my ($self, $msg) = @_; $self->fh->print($msg."\n") if $self->log(); print($msg."\n") if $self->shell(); }
Da sieht man beim Lesen des Codes direkt, was du an Argumenten erwartest.
Das macht später die Pflege des Codes deutlich einfacher.