Best Practice Klasse mit Array clonen

[Holger_Hessling]

Hallo Forum,

folgende Klasse bietet an, Instanzen von ihr zu clonen:

public class ProfilPlan extends DOElement implements Cloneable {

   private ObservableList<PlanZeile> profilPlan = FXCollections.observableArrayList();

   public ProfilPlan(String label) {
     super(label);
   }

   @Override
   public ProfilPlan clone() {
     ProfilPlan klon = (ProfilPlan) super.clone();

     klon.profilPlan = FXCollections.observableArrayList(profilPlan);
     for (PlanZeile planZeile : klon.profilPlan) {
       klon.profilPlan.set(klon.profilPlan.indexOf(planZeile),planZeile.clone());
     }
     klon.setLabel(klon.toString() + " - ProfilPlan clone");
     return klon;
   }
}

Um das Array zu klonen, bekommt der Klon in einem ersten Schritt ein neues Array, das dem Originalen entspricht. Um eine 'tiefe Kopie' zu erhalten, laufe ich durch dieses Array und klone jedes einzelne Element.
Das funktioniert, aber es sieht holprig aus. Geht das auch eleganter?

VG Holger

 

[thet1983]

Meine Meinung ist, ein Code ist da um zu funktionieren und nicht um einen Schönheitswettbewerb zu gewinnen....solange er das tut für was er geschrieben wurde, brauchst du ja nichts ändern, oder?

 

[Holger_Hessling]

Das hast Du prinzipiell recht

 

[VfL_Freak]

Moin,

Meine Meinung ist, ein Code ist da um zu funktionieren und nicht um einen Schönheitswettbewerb zu gewinnen....solange er das tut für was er geschrieben wurde, brauchst du ja nichts ändern, oder?

na ja, ganz so würde ich es nicht sehen!

Es sollten schon gewisse Style-Guides eingehalten werden.

So dann ist IMHO nur lesbarer Code guter Code - vorallem wenn man im Team an größeren Projekten arbeitet! Es bringt nix, wenn es 'irgendwie' zu funktionieren scheint, aber kein andere versteht, was da eigentlich passiert ....

Gruß Klaus

 

[Meeresgott]

@Holger_Hessling
Der Code ist gut so wie er ist. Mann sieht schön das du das Objekt clonst und dann jedes andere Objekt in der Liste.
Mir würde auch kein anderer Weg einfallen das zu realisieren außer mit Streams aber da würdest du ja das selbe machen

Oder deine Clone Array Methode auslagern aber so wie es jetzt ist finde ich es am Leserlichsten

Viele Grüße

 

[Flown]

Ich finde es nicht so clean wenn man indexOf etc einsetzen muss. Ich würde das eher so machen:

ObservableList<PlanZeile> list = FXCollections.observableArrayList();
for(PlanZeile planZeile : profilPlan) {
  list.add(planZeile.clone());
}
klon.profilPlan = list;

Oder eben mit dem ListIterator:

klon.profilPlan = FXCollections.observableArrayList(profilPlan);
for(ListIterator<PlanZeile> lit = klon.profilPlan.listIterator(); lit.hasNext()) {
  lit.set(list.next().clone());
}

PS: Beide Codes sind ungetestet, hab gerade keine IDE.

 

[thet1983]

@VfL_Freak , sicher muss die formatierung passen, aber so einen code könnte ein blinder mit seinem hühnerauge lesen, oder?

ich würde nichts daran ändern, doc hinzu und fertig..

 

[Holger_Hessling]

Hallo Florian,

Danke!! Dein erster Vorschlag gefällt mir sehr gut. Einfach eine neue Liste aufbauen entzerrt die Sache ungemein. Gute Idee!

VG Holger

 

[VfL_Freak]

@VfL_Freak , sicher muss die formatierung passen, aber so einen code könnte ein blinder mit seinem hühnerauge lesen, oder?
ich würde nichts daran ändern, doc hinzu und fertig..

War auch mehr eine allgemeine Bemerkung