littlemight/Avatar-Duel

Singleton on Dealer

Closed this issue · 4 comments

I think this may lead to performance issue. If the Dealer constructed multiple times, then the csv will be loaded again and again. Isn't this bad? Instead, why don't we use singleton design pattern here?

public class Dealer {
private List<Card> cards;
private static final String LAND_CSV_FILE_PATH = "../card/data/land.csv";
private static final String CHARACTER_CSV_FILE_PATH = "../card/data/character.csv";
private static final String SKILL_AURA_CSV_FILE_PATH = "../card/data/skill_aura.csv";
public Dealer(){
try{
this.loadCards();
}
catch(Exception e){
System.out.println("Failed to load cards: " + e);
}
}

This issue is only a suggestion. You may argue with it (not all the suggestions are true), challenge it, or come up with another idea yourself.

Yes, I agree. Thank you for the suggestion.

On second thought, singleton is bad. This stackoverflow answer will explain why. Instead, why don't we inject the dealer to player, instead constructing it?

Actually, the only time dealer will be constructed is when the game is in setup mode. It serves to prepare the deck for the player and will never need to be declared again, so I think if we don't use singleton and just declaring it and using it in the main class (player will accept the deck), it won't violate anything. Thank you for pointing out that the dealer is constructed more than once.

What you mean by

(player will accept the deck)
is injecting the deck into player. It is the example of Dependency Injection Principle. So I think that's good.