Checkstyle's understanding of Enums is sometimes incomplete. I would guess that the author of the VisibilityModifier check did not think of Enums. Enums are a corner case that would require some additional properties of the check.
However, accidentally, in your case, the warning is still correct. I believe that there is no situation where you should use protected
fields in an Enum. If you need instance specific state in your Enum constants, initialize it via the constructor or via static initialization.
Try the following approach. This should gain you several benefits:
- The Checkstyle warning goes away because the
Map
is nowprivate
. - The
CatanPiece
enum constants are now really constants because the cost map is immutable. - No overhead for lazy initialization. Users of the
CatanPiece
enum constants can be sure that the costs map is initialized properly.
The only downside is that whenever the Enum is extended (e.g. with SHIP
), the developer must not forget to update buildCostMap()
. Such a bug would show up very quickly though.
public enum CatanPiece implements Buildable
{
ROAD, SETTLEMENT, CITY;
private static final Map<CatanPiece, Map<Resource, Integer>> allCosts =
buildCostMap();
private static Map<CatanPiece, Map<Resource, Integer>> buildCostMap()
{
Map<CatanPiece, Map<Resource, Integer>> result =
new HashMap<CatanPiece, Map<Resource, Integer>>();
Map<Resource, Integer> cost = new EnumMap<Resource, Integer>(Resource.class);
cost.put(Resource.WHEAT, 2);
cost.put(Resource.ORE, 3);
result.put(CITY, Collections.unmodifiableMap(cost));
cost = new EnumMap<Resource, Integer>(Resource.class);
cost.put(Resource.BRICK, 1);
cost.put(Resource.LUMBER, 1);
cost.put(Resource.SHEEP, 1);
cost.put(Resource.WHEAT, 1);
result.put(SETTLEMENT, Collections.unmodifiableMap(cost));
cost = new EnumMap<Resource, Integer>(Resource.class);
cost.put(Resource.BRICK, 1);
cost.put(Resource.LUMBER, 1);
result.put(ROAD, Collections.unmodifiableMap(cost));
return Collections.unmodifiableMap(result);
}
@Override
public Map<Resource, Integer> getCost() {
return allCosts.get(this);
}
@Override
public Buildable build(final PlayerHand payment) {
return payment.remove(cost) ? null : this;
}
}