static arraylist with instances of Class with initial values

Question

Im making a Coin class with a static arraylist that stores every instance of the class created, howevered I need to initiate that list with an initial instance, and I have not figured out how to do it without adding it twice (because of a redundant code), any suggestions?

public class Coin {
    private static ArrayList<String> coinNames = new ArrayList<>();
    private static ArrayList<String> coinAbbreviations = new ArrayList<>(Arrays.asList("CLP"));
    private static ArrayList<Coin> coins =
            new ArrayList<>(Arrays.asList(new Coin("Pesos chilenos", "CLP", 1f, "CLP")));
    private static HashMap<String,Float> exchangeRates;
    private String coinName;
    private String coinAbbreviation;
    private Float coinValue;
    private String unit;


    public Coin(String coinName, String coinAbbreviation, Float coinValue, String unit) {
        assert !coinAbbreviations.contains(coinAbbreviation) : "Coin abbreviation already used";
        assert coinAbbreviations.contains(unit) : "Coin unit non existent.";
        assert !coinNames.contains(coinName) : "Coin name already used.";
        this.coinName = coinName;
        this.coinAbbreviation = coinAbbreviation;
        this.coinValue = coinValue;
        this.unit = unit;

        coins.add(this);
    }
}

*static arraylist that stores every instance* not sure what are your trying to do — Ravi, Sep 19 '17 at 17:31
Please keep in mind that this pattern is inherently thread-unsafe (that is, you can basically never make thread-safe code with it), so it's a bad habit to get into. A better approach would be a private constructor, and a static factory method that calls that constructor and then adds the instance to `coins`. — yshavit, Sep 19 '17 at 17:31
Better would be to have a CoinFactory class which creates (and returns) Coin instances and adds them to itself. Your current solution is not thread-safe and that's just one drawback. — DodgyCodeException, Sep 19 '17 at 17:35
@DodgyCodeException could you explain please, i think that might be the solution — Nicolas Quiroz, Sep 19 '17 at 17:40
@NicolasQuiroz I've tried to explain; see my new answer below. — DodgyCodeException, Sep 20 '17 at 10:22

score 5 · Answer 1 · edited Sep 19 '17 at 17:34

5

If you insist on having mutable static variables at all -- it's generally not a good idea to do things like this at all -- you could do

private static ArrayList<Coin> coins =
        new ArrayList<>();

static {
  new Coin("Pesos chilenos", "CLP", 1f, "CLP");
}

...which adds the element to the list immediately.

edited Sep 19 '17 at 17:34

azro

53,056
7
34
70

answered Sep 19 '17 at 17:31

Louis Wasserman

191,574
25
345
413

what is a better idea then? – Nicolas Quiroz Sep 19 '17 at 17:40
4

Have an actual class that stores all the coins you make. Don't make it tied to the constructor, list them all explicitly. – Louis Wasserman Sep 19 '17 at 17:41
Hey I dont get the part when you said _Don't make it tied to the constructor, list them all explicitly_ . By that you mean that whenever I instatiate it, the constructor won't automatically store it, but I have to do it manually? – Nicolas Quiroz Sep 19 '17 at 23:18
3

Yes. Having your constructor automatically store things is dangerous and worrisome. – Louis Wasserman Sep 19 '17 at 23:23

score 1 · Answer 2 · answered Sep 19 '17 at 17:34

1

What stops you initialising your list in its declaration and then just adding each instance to the list in the constructor?

answered Sep 19 '17 at 17:34

user8635668

31
3

2

That's what the OP is doing, but what they want is for one Coin (the "Pesos chilenos") one to be "automatically" added, without the user of this class having to manually instantiate it. – yshavit Sep 19 '17 at 17:35

DodgyCodeException · Accepted Answer · 2017-09-21T14:43:12.513

You could alternatively design your application using some best-practice patterns. You want to keep a registry of all created coins. This is best kept outside of the Coin class itself. You could have a class that manages the creation of coins and keeps a list of those that it created. The Coin class itself can be an interface, if you like, as that way you ensure that it cannot be created other than by the CoinFactory.

public interface Coin {
    String name();
    String abbreviation();
    BigDecimal value();
    String unit();
}

And the Coin factory class:

public class CoinFactory {

    // Concrete coin is an internal implementation class whose details don't
    // need to be known outside of the CoinFactory class.
    // Users just see it as interface Coin.
    private static class ConcreteCoin implements Coin {
        private final String name;
        private final String abbreviation;
        private final BigDecimal value;
        private final String unit;

        ConcreteCoin(String name, String abbreviation, BigDecimal value, String unit) {
            this.abbreviation = abbreviation;
            this.name = name;
            this.value = value;
            this.unit = unit;
        }

        public String name() { return name; }
        public String abbreviation() { return abbreviation; }
        public BigDecimal value() { return value; }
        public String unit() { return unit; }
    }

    // Sets for enforcing uniqueness of names and abbreviations
    private Set<String> names = new HashSet<>();
    private Set<String> abbreviations = new HashSet<>();

    // All coins must have one of the following ISO currency codes as the 'unit' field.
    private final Set<String> allIsoCurrencyCodes =
            Set.of("CLP", "GBP", "EUR", "CAD", "USD", "XXX" /* , ... */);

    private List<Coin> allCoins = new ArrayList<>(
            List.of(createCoin("Pesos chilenos", "CLP", BigDecimal.ONE, "CLP")));

    private List<Coin> unmodifiableListOfAllCoins =
            Collections.unmodifiableList(allCoins);


    public Coin createCoin(String name, String abbreviation, BigDecimal value, String unit) {
        if (!names.add(name))
            throw new IllegalArgumentException("Name already exists: " + name);
        if (!abbreviations.add(abbreviation))
            throw new IllegalArgumentException("Abbreviation already exists: " + abbreviation);
        if (!allIsoCurrencyCodes.contains(unit))
            throw new IllegalArgumentException("Coin unit is not a recognised ISO currency code: " + unit);

        Coin coin = new ConcreteCoin(name, abbreviation, value, unit);
        allCoins.add(coin);
        return coin;
    }

    public Collection<Coin> allCoins() {
        return unmodifiableListOfAllCoins;
    }
}

static arraylist with instances of Class with initial values

3 Answers3