Euromünzen in enum als Liste ausgeben

[kilopack15]

Hallo Leute,
ich habe folgende Aufgabe gelöst und wollte mal euer Feedback zu meiner Lösung haben, da ich nun das erste Mal mit enum und LinkedList programmiert habe.
Die Aufgabe:
a.)Implementieren Sie zur Repräsentation von Euro-Münzen einen Aufzählungstyp (Enum) Muenze! Der Aufzählungstyp enthält für jede Euro-Münze genau ein Element:
EinCent, ZweiCent, FuenfCent, ZehnCent ZwanzigCent, FuenfzigCent, EinEuro, ZweiEuro
Implementieren Sie außerdem eine Methode int getValue(), die den Wert einer
Euro-Münze (in Cent) liefert!

b) Implementieren Sie folgende statische Methode: public static LinkedList<Muenze> getMuenzen(int wertInCent) Diese liefert eine Liste von Euro-Münzen,sodass der Wert aller Münzen zusammen dem übergebenen nicht negativen Betrag (in Cent) entspricht! Beachten Sie dabei, dass die Liste nur eine minimale Anzahl an Münzen enthalten darf! Für negative Werte soll eine IllegalArgumentException geworfen werden!

Meine Lösung:

import java.util.LinkedList ;
import java.lang.IllegalArgumentException;
//a.)
public enum Muenze {
    //Aufzählen der Elemente von Muenze
    EinCent, ZweiCent, FuenfCent, ZehnCent,
    ZwanzigCent, FuenfzigCent, EinEuro, ZweiEuro;
 
 
    /** Methode gibt den Wert in Cent einer Münze zurück
     * @param
     * @return Wert der Münze in Cent
     */
    int getValue(){
        switch(this){
        case EinCent: return 1;
        case ZweiCent: return 2;
        case FuenfCent: return 5;
        case ZehnCent: return 10;
        case ZwanzigCent: return 20;
        case FuenfzigCent: return 50;
        case EinEuro: return 100;
        case ZweiEuro: return 200;
        default: return 0;
        }
    }
  
    // b.)
    /**
     * Methode erstellt eine Liste von Münzen, die zusammen addiert Parameter wertInCent ergeben.
     * @param wertInCent
     * @return Es wird eine Liste zurückgegeben, die die Münzen enthält, die addiert den wertInCent
     * ergeben.
     */

    public static LinkedList<Muenze> getMuenzen(int wertInCent){
        //Exception, falls wertInCent negativ
        if(wertInCent < 0){
            throw new IllegalArgumentException();
        }
        //Erstellen einer neuen LinkedList
        LinkedList<Muenze> list = new LinkedList<Muenze>();
     
        /*während der wertInCent größer als 200 Cent ist, wird eine Zwei-Euro-Münze zur
        Liste hinzugefügt. Analog bei den weiteren Münzen
        */
     
        while(wertInCent>=200){
            list.add(ZweiEuro);
            wertInCent -= 200;
        }
     
        while(wertInCent>=100){
            list.add(EinEuro);
            wertInCent -= 100;
        }
     
        while(wertInCent>=50){
            list.add(FuenfzigCent);
            wertInCent -= 50;
        }
     
        while(wertInCent>=20){
            list.add(ZwanzigCent);
            wertInCent -=20;
        }
     
        while(wertInCent>=10){
            list.add(ZehnCent);
            wertInCent -=10;
        }
     
        while(wertInCent>=5){
            list.add(FuenfCent);
            wertInCent -= 5;
        }
     
        while(wertInCent>=2){
            list.add(ZweiCent);
            wertInCent -=2;
        }
     
        while(wertInCent>=1){
            list.add(EinCent);
            wertInCent -=1;
        }
        //Ausgeben der Liste
        if(wertInCent == 0){
            return list;
        }
        return list;
    }
}

Habe ich die Aufgabe richtig gelöst?

 

[AndyJ]

Der switch in dem Enum ist keine gute Idee. Ein enum Typ ist ja eine vollstaendige Klasse, also kannst du die gestalten wie du willst. Der individuelle Wert ist ja der Cent-Betrag, also kannst du den deinen Objekten mitgeben:

public enum Muenze {
  EinCent(1),
  ZweiCent(2),
  FuenfCent(5),
  ZehnCent(10),
  ZwanzigCent(20),
  FuenfzigCent(50),
  EinEuro(100),
  ZweiEuro(200);

  private final int value;

  private Muenze(int value) {
  this.value = value;
  }

  public int getValue() {
  return this.value;
  }

}

Weiter unten im Code hast du noch folgende Statements:

   //Ausgeben der Liste
   if(wertInCent == 0){
     return list;
   }
   return list;

Da wertInCent an dieser Stelle nur 0 sein kann, genuegt das return list; Statement.

Andy

 

[stg]

Richtig gelöst ist sie erst einmal.

Zusätzlich zu den bereits gemachten Anmerkungen, sollten dir aber auch deine zahlreichen komplett gleich aufgebauten while-Schleifen ins Auge springen. Das schreit nach Verbesserung.
Ich würde hier der Größe nach absteigend über alle Münzen iterieren, und dann für jede einzelne Münze .... du kommst bestimmt selbst drauf.
Somit eliminierst du all deine Wiederholungen im Code und kommst mit etwa 30 Zeilen weniger Code aus.

 

[Jardcore]

Enums haben noch mehr möglichkeiten dein Problem zu lösen. Zu jedem Wert können noch beliebige Parameter hinzugefügt werden. Dafür benötigst du einen Konstruktor und private Attribute.

Im folgenden Beispiel wurde der Wert einer Münze als Parameter hinzugefügt.

public enum Muenze {
    ZweiEuro(200),
    EinEuro(100),
    FuenfzigCent(50),
    ZwanzigCent(20),
    ZehnCent(10),
    FuenfCent(5),
    ZweiCent(2),
    EinCent(1);
   
    private int valueInCent;
    private Muenze(int valueInCent) {
        this.valueInCent = valueInCent;
    }
   
    public int getValue() {
        return valueInCent;
    }
 
    public static LinkedList<Muenze> getMuenzen(int wertInCent){

        if(wertInCent < 0){
            throw new IllegalArgumentException();
        }
       
        LinkedList<Muenze> minimalListOfCoins = new LinkedList<Muenze>();
    
        // solange weiter machen, bis der wertInCent 0 ist
        while (wertInCent != 0) {
            // alle Enumwerte durchgehen, angefangen bei ZweiEuro.
            for(Muenze muenze : Muenze.values()) {
                /* Wenn der übergebene Cent-Wert größer oder gleich dem aktuellen Muenzwert ist
                 * wird der aktuelle Meunzwert zur Liste hinzugefügt und der Wert vom übergebenen wertInCent abgezogen.
                 * Damit wieder möglichst wenig Münzen gebildet werden, wird die Schleife dann unterbrochen und es wird
                 * wieder bei ZweiEuro gestartet.
                 */
                if(wertInCent >= muenze.getValue()) {
                    minimalListOfCoins.add(muenze);
                    wertInCent -= muenze.getValue();
                    break;
                }
            }
        }

        return minimalListOfCoins;
    }
}

 

[X5-599]

Hab ein wenig Langeweile. Ich hätte es wohl so gemacht:

public static List<Muenze> getMuenzen(int amountCents)
{
	List<Muenze> muenzen = new ArrayList<Muenze>();
	
	for(Muenze muenze : Muenze.values())
	{
		int nCoins = amountCents / muenze.getValue();
		for(int i = 0; i < nCoins; i++)
		{
			muenzen.add(muenze);
		}
		
		amountCents = amountCents % muenze.getValue();
	}
	
	return muenzen;
}

 

[Jardcore]

Sehr gut, hatte mir schon gedacht, das es etwas besser geht.
Es muss aber hier erwähnt werden, das die Reihenfolge des Enums bei meinem und dem letzten Vorschlag extrem wichtig ist.

 

[X5-599]

Da hast du natürlich recht. Ist eigentlich irgendwo definiert wie das Enum#values() sein Array erstellt? Um sicher zu gehen kann man aber auch sein eigenes Array erstellen. So z.B.:

public static List<Muenze> getMuenzen(int amountCents)
{
	List<Muenze> muenzen = new ArrayList<Muenze>();
	
	Muenze[] all = new Muenze[] {
			Muenze.ZWEI_EURO,
			Muenze.EIN_EURO,
			Muenze.FUENFZIG_CENT,
			Muenze.ZWANZIG_CENT,
			Muenze.ZEHN_CENT,
			Muenze.FUENF_CENT,
			Muenze.ZWEI_CENT,
			Muenze.EIN_CENT
	};
	
	for(int i = 0; i < all.length; i++)
	{
		Muenze muenze = all[i];
		
		int nCoins = amountCents / muenze.getValue();
		for(int j = 0; j < nCoins; j++)
		{
			muenzen.add(muenze);
		}
		
		amountCents = amountCents % muenze.getValue();
	}
	
	return muenzen;
}

 

[Flown]

Ist eigentlich irgendwo definiert wie das Enum#values() sein Array erstellt?

JLS ist immer ein guter Tipp zum nachschlagen. Aber HIER der 4te Punkt.

 

[X5-599]

Danke für den Tipp. Normalerweise reicht mir ein Blick in die API Doku. Leider verlinkt Eclipse bei der Enum#values() Methode nichts...

>> "Returns an array containing the constants of this enum type, in the order they're declared."

Da also tatsächlich die Rückgabe Reihenfolge von Enum#values() garantiert ist, würde ich meine Variante aus Post #5 vorziehen. Weil: weniger Zeilen Code und übersichtlicher. Aber natürlich wie @Jardcore schon sagte: Die Reihenfolge in der man die Elemente des Muenze Enums anlegt ist sehr wichtig.

 

[AndyJ]

Ein Algorithmus sollte sich nicht auf die Order der enum values verlassen. Die kann sich naemlich jederzeit aendern, wenn jemand die enums neu anordnet und dann funktioniert der Algorithmus nicht mehr. Solche Fehler sind dann schwer zu finden. Besser waere es vielleicht, eine statische Methode hinzuzufuegen die die enums sortiert nach Wert zurueckliefert.

 

[Jardcore]

Folgenden Code könnte man dazu benutzen die Muenzen immer absteigend zu bekommen.

    public static List<Muenze> getCoinsDescending() {
        return Arrays.asList(Muenze.values()).stream()
                .sorted((c1, c2) -> Integer.compare(c2.getValue(), c1.getValue()))
                .collect(Collectors.toList());
    }

 

[Flown]

@Jardcore besser wäre dann noch:

public static List<Muenze> getSortedList() {
  return Collections.unmodifiableList(Arrays.stream(values())
      .sorted(Comparator.comparingInt(Muenze::getValue).reversed()).collect(Collectors.toList()));
}