Skip to content
Browse files

review Kommentare ergaenzt

  • Loading branch information...
1 parent 322a0d2 commit d3679c4481d6707404519795d7b40831ac818cec @schauder committed Nov 24, 2013
Showing with 12 additions and 2 deletions.
  1. +5 −0 review.md
  2. +2 −0 src/businessLogic/SaveCDPersistent.java
  3. +1 −0 src/gui/CreateCDFail.java
  4. +1 −1 src/gui/CreateCDForm.java
  5. +3 −1 src/persistenz/CDStore.java
View
5 review.md
@@ -0,0 +1,5 @@
+compilierte Artefakte(bin Ordner) und Backup Dateien (README.md~), die von Editoren angelegt werden, sollten nicht Versionsverwaltet werden, da sie aus versionsverwalteteten Resourcen erzeugt werden können, bzw. für das Projekt nicht relevant sind.
+
+Die Namen der Klassen sind etwas strukturlos, und müssten stark überarbeitet werden.
+
+Ich habe die Klassen einzeln mit Reviewkommentaren versehen, dabei ist nur einer der sich auf ein Problem mit der Architektur bezieht.
View
2 src/businessLogic/SaveCDPersistent.java
@@ -34,7 +34,9 @@ public void save(CD cd, JFrame cdf){
if(!this.persistenzMemory.lookForDuplicate(cd)){
this.persistenzMemory.save(cd);
} else {
+ // REVIEW die Warnung hat durchaus einen Sinn. Da cdFail nicht verwendet wird, kann die Variable und damit die Zuweisung entfallen.
@SuppressWarnings("unused")
+ //REVIEW an dieser Stelle referenziert die businessLogic die gui, ein austauschen der GUI wird dadurch nur schwer möglich. Um der Architektur gerecht zu werden müsste die Methode save einen Rückgabewert haben, der dann von der GUI ausgewertetet wird. Das Äquivalent der folgenden Code Zeile müsste dann dort stehen.
CreateCDFail cdFail = new CreateCDFail(cdf);
}
View
1 src/gui/CreateCDFail.java
@@ -25,6 +25,7 @@
public CreateCDFail(JFrame jf) {
super();
this.setSize(100, 100);
+ // REVIEW ein Fenster im Konstruktor anzuzeigen ist eine schlechte Idee, da es keine Möglichkeit mehr gibt diese Klasse ohne Oberfläche zu testen. Es wäre daher besser eine Methode anzubieten, mit der der Dialog angezeigt wird.
this.showMessageDialog(jf,"CD already exists in your library!", "CD save error", JOptionPane.ERROR_MESSAGE);
}
}
View
2 src/gui/CreateCDForm.java
@@ -33,7 +33,7 @@
public CreateCDForm(SaveCDPersistent scdp) {
this.saveCDPersistent = scdp;
this.setLayout(new GridLayout(3, 2));
-
+ // REVIEW die durch Leerzeilen getrennten Blöcke deuten darauf hin, dass hier Methoden extrahiert werden möchten.
this.titleLabel = new JLabel("Titel:");
this.add(this.titleLabel);
this.titleTextField = new JTextField();
View
4 src/persistenz/CDStore.java
@@ -7,7 +7,7 @@
import businessLogic.PersistenzMemoryInterface;
public class CDStore implements PersistenzMemoryInterface{
-
+ // REVIEW cdList sollte als List, nicht als ArrayList deklariert werden, da die exakte implementierung keine Rolle spielt
private ArrayList<CD> cdList;
public CDStore(){
@@ -19,9 +19,11 @@ public void save(CD cd){
}
public boolean lookForDuplicate(CD cd){
+ // REVIEW hier wäre eine vor each Schleife bessert
for (Iterator<CD> iterator = cdList.iterator(); iterator.hasNext();) {
CD object = (CD) iterator.next();
+ // REVIEW für einen Vergleich auf Gleichheit sollte equals verwendet werden
if(object.compareTo(cd) == 0){
return true;
}

0 comments on commit d3679c4

Please sign in to comment.
Something went wrong with that request. Please try again.