Clanintern Clanintern Clanintern

Forum

Öffentliche Foren
ZURÜCK
FORUM: Spiele & Computer THEMA: Refactoring JAVA
AUTOR BEITRAG
DoomTheBrain

RANG Skill Apprentice

#1 - 17.05 20:10

ja habe mol eine kurze JAVA frage... bin gerade am code nochmals durchschauen und bisschen duplicated code am entfernen.... dann diese stelle:

code:


private void generateCoachCards() {
int coachSize = 3; // 2 Punkte
for (int i = 0; i < 4; i++)
items.add(new CoachCard( coachSize,2 ));

coachSize = 4; // 3 Punkte
for (int i = 0; i < 4; i++)
items.add(new CoachCard( coachSize,2 ));

coachSize = 5; //5 Punkte
for (int i = 0; i < 4; i++)
items.add(new CoachCard( coachSize,5 ));

coachSize = 6; // 7 Punkte
for (int i = 0; i < 4; i++)
items.add(new CoachCard( coachSize,7 ));

coachSize = 7; // 10 Punkte
for (int i = 0; i < 4; i++)
items.add(new CoachCard( coachSize,10 ));
}



müsste ja eigentlich mit einer zweiten for schelife und dan nnur einmal die zeile

items.add usw....

lösbar sien. allerdings föllt mir einfach nichts richtiges ein ... hat jemand eine idee?

thx
Crush (sexy shoeless god of war)

RANG Deckschrubber

#2 - 17.05 20:50

Spielt es eine Rolle in welcher Reihenfolge die Elemente "items" hinzugefügt werden?
DoomTheBrain

RANG Skill Apprentice

#3 - 17.05 21:03

ja muss diese reihenfolge sein
NeDoH-*mit Wii*

RANG Deckschrubber

#4 - 17.05 21:05

Ach die dummen Zahlen hängen mit der anderen Frage nach der Reihe zusammen?

Hier ist aber schonmal ein Fehler, entweder im Kommentar oder im Code, egal, beides gleich schlimm:
" coachSize = 4; // 3 Punkte
for (int i = 0; i < 4; i++)
items.add(new CoachCard( coachSize,2 ));"
Entweder 2 Punkte, oder du rufst CoachCard falsch auf.

Ansonsten ist dein Problem ja nur wie die dummen Punkte bei CoachCard bestimmt werden, also entweder hast du da ne Formel (also ich mein, woher kommt das Zeug?) oder du schreibst dir einfach ne eigene Funktion mit switch-cases. Für 5 Werte noch kein Problem.
DoomTheBrain

RANG Skill Apprentice

#5 - 17.05 21:14

danke hoden das hötte einen bösen fehler geben können

muss 3 sein im coachsize
DoomTheBrain

RANG Skill Apprentice

#6 - 17.05 22:32

also habe mal noch bisshcen weiter überlegt und bin dann auf diese lösung gekommen.... ist wohl klar dass es nicht wirklich sinnvol ist diese lange formel zu berechnen, aber wenn es jetzt theoretisch wirklich nur um die anzahl codezeilen geht müsste die lösung doch stimmen odr?

code:

private void generateCoachCards() {
for (int i = 3; i < 8, i++) {
for (int i = 0; i < 4; i++)
items.add(new CoachCard( i, 1/12 * i^4 - 5/3 * i^3 + 149/12 * i^2 + 233/6 * i + 45 ));
}
}
Crush (sexy shoeless god of war)

RANG Deckschrubber

#7 - 18.05 12:24

Das mit der for Schleife von 3 bis 8 kann man machen. Aber durch diese lange Formel wird der Code zwar kürzer aber dafür unlesbar und unperformanter. Ist das wirklich eine Verbesserung?

Achja, und die zweite for Schleife sollte vielleicht besser eine eigene Variable bekommen :)

Was ich sonst am Code im ersten Post nicht verstehe, ist warum du das erste Argument von CoachCard immer erst in einer Variable zwischenspeicherst und nicht direkt übergibst.
DoomTheBrain

RANG Skill Apprentice

#8 - 18.05 12:26

ja das wurde wohl zur besseren übersicht gemacht keine ahnung

stimmt die zweite forschlaufe ist dann j oder so
Morath

RANG Deckschrubber

#9 - 21.05 08:30

Das zweite Argument vom CoachCard-Konstruktor (die Punkte) scheint ja direkt abhängig von der coachSize zu sein, ist es denn dann überhaupt nötig, an CoachCard beide Werte zu übergeben?

Ein Refactoring innerhalb des CoachCard-Konstruktors könnte die Anzahl der Punkte ja abhängig von der übergebenen coachSize ermitteln, z.B. mit einem statischen Array:

code:

// die Punkte für coachSize ab 3, die unbenutzten Indizes 0-2 werden mit 0 belegt
private static final int[] PUNKTE = { 0, 0, 0, 2, 3, 5, 7, 10 };


Falls das nicht möglich ist, kann das Array auch in der Klasse benutzt werden, die den Schleifencode enthält:

code:

for (int coachSize = 3; coachSize < 8; coachSize++) {
  for (int i = 0; i < 4; i++) {
    items.add(new CoachCard( coachSize, PUNKTE[coachSize] ));
  }
}
DoomTheBrain

RANG Skill Apprentice

#10 - 21.05 10:46

hey cool danke.. das scheint mir mal eine gute lösung zu sein
DoomTheBrain

RANG Skill Apprentice

#11 - 21.05 11:35

ja und weil ihr da so gut seid gerade noch ein beispiel

code:


private void generateBonusChips() {


int routeLength = 5; // 1,2 Punkte
for (int i = 0; i < 2; i++)
items.add(new RouteChip( routeLength, i+1 ));

routeLength = 6; // 1,2,3 Punkte
for (int i = 0; i < 3; i++)
items.add(new RouteChip( routeLength, i+1 ));

routeLength = 7; // 1,2,3,4 Punkte
for (int i = 0; i < 4; i++)
items.add(new RouteChip( routeLength, i+1 ));


int country = 1; 
for (int i = 0; i < 4; i++)
items.add(new CountryChip( country, i+2 ));

country = 2;  
for (int i = 0; i < 4; i++)
items.add(new CountryChip( country, i+3 ));

country = 3;  
for (int i = 0; i < 3; i++)
items.add(new CountryChip( country, i+2 ));

country = 4;  
for (int i = 0; i < 3; i++)
items.add(new CountryChip( country, i+1 ));

country = 5; 
for (int i = 0; i < 3; i++)
items.add(new CountryChip( country, i+1 ));

country = 6; 
for (int i = 0; i < 3; i++)
items.add(new CountryChip( country, i+1 ));
                }
         }
Morath

RANG Deckschrubber

#12 - 21.05 13:05

Prinzipiell lässt sich das ähnlich lösen, ist aber immer die Frage wie sinnvoll sowas für die Lesbarkeit und spätere Wartbarkeit ist.

Den Teil mit den RouteChips kannst du auch problemlos so lassen wie er ist, du kannst ihn aber natürlich auch in eine rein berechnete Form bringen, z.B.

code:

for (int routeLength = 5; routeLength < 8; routeLength++) {
  for (int punkte = 1; punkte < routeLength - 2; punkte++) {
    items.add(new RouteChip( routeLength, punkte ));
  }
}


ob das aber wirklich so leicht verständlich ist (insbesondere die Abbruchbedingung der inneren Schleife "routeLength - 2", die einfach die Abhängigkeit der Punktanzahl von routeLength ausdrückt), wage ich mal zu bezweifeln.

Solche kleinen Initialisierungen fallen von der Laufzeit her aber sowieso nicht merklich ins Gewicht, so dass du genausogut einen Block der Form

code:

// routeLength = 5, 1-2 Punkte
items.add(new RouteChip( 5, 1 ));
items.add(new RouteChip( 5, 2 ));

// routeLength = 6, 1-3 Punkte
items.add(new RouteChip( 6, 1 ));
items.add(new RouteChip( 6, 2 ));
items.add(new RouteChip( 6, 3 ));

// routeLength = 7, 1-4 Punkte
items.add(new RouteChip( 7, 1 ));
items.add(new RouteChip( 7, 2 ));
items.add(new RouteChip( 7, 3 ));
items.add(new RouteChip( 7, 4 ));


schreiben könntest, was meiner Meinung nach viel eher verständlich ist, als die beiden Verschachtelten Schleifen. Wobei ich den usprünglichen Codeblock auch noch für übersichtlich genug halte um sofort zu erkennen, worum es da eigentlich geht.


Bei den CountryChips liegt das Problem darin, dass sich sowohl der Startwert der Punkte als auch die Anzahl der zu generierenden Chips abhängig vom country verändert. Eine mögliche Lösung wären zwei Arrays (bzw. ein zweidimensionales Array, was mir persönlich aber nicht so gefällt):

code:

private static final int[] COUNTRY_CHIP_COUNT = { 0, 4, 4, 3, 3, 3, 3 };
private static final int[] COUNTRY_CHIP_START_VALUE = { 0, 2, 3, 2, 1, 1, 1 };


Damit lässt sich das Ganze wieder mit zwei verschachtelten Schleifen ausdrücken:

code:

for (int country = 1; country < 7; country++) {
  for (int value = 0;
        value < COUNTRY_CHIP_COUNT[country]; value++) {
    items.add(new CountryChip( country, 
        value + COUNTRY_CHIP_START_VALUE[country] ));
  }
}


Find ich aber auch nicht sonderlich intuitiv zu lesen...
DoomTheBrain

RANG Skill Apprentice

#13 - 21.05 14:07

morath scheinst ja ein richtiger experte zu sein! danke dir
Morath

RANG Deckschrubber

#14 - 21.05 15:35

hehe, danke :-)

Ich steck berufsbedingt da ein "klein wenig" drin, ja ^^
ZURÜCK