Nsryjdtyk

i'm searching the smartest way of handle this method

public boolean addAccount(String cf, AccountType type) {

    String iban = name + cf;

    if (bankAccounts.containsKey(iban)) return false;

    if (type.name().equals("CHECKINGACCOUNT")) {

        CheckingAccount cc = new CheckingAccount(cf, iban, 0);

        bankAccounts.put(iban, cc);

        return true;

    }

    if (type.name().equals("DEPOSIT")) {

        DepositAccount cd = new DepositAccount(cf, iban, 0);

        bankAccounts.put(iban, cd);

        return true;

    }

    if (type.name().equals("WEB")) {

        WebAccount cw = new WebAccount(cf, iban, 0);

        bankAccounts.put(iban, cw);

        return true;

    }

  return false;

}

AccountType is enum that contains (DEPOSIT,WEB,CHECKINGACCOUNT);
bankAccounts is an HashMap that contains iban (key) & CheckingAccounts OR DepositAccount OR WebAccount;
CheckingAccounts ,DepositAccount,WebAccount are 3 classes that inherit an abstract class called Account.

I'm trying to substitute the if with an HashMap that check the key (Type of account) with the String insert by the user and instantiates one of the three class associated to the key in the HashMap.
The problem is that i can't create the correspondence between the String and the Account because i need to instantiate that (but i don't know the cf in that moment)

Could someone show me some better way to manage it?

Why not put the logic for which Account to create right in the enum itself, using a factory pattern? Ever since Java 8, this pattern is really slick, as you can literally pass in the constructor as an implementation of the factory:

public boolean addAccount(String cf, AccountType type) {

    String iban = name + cf;

    if (bankAccounts.containsKey(iban)) return false;

    Account account = type.createAccount(cf, iban, 0);    

    bankAccounts.put(iban, account);

    return true;

}



public enum AccountType {

    CHECKING(CheckingAccount::new),

    DEPOSIT(DepositAccount::new),

    WEB(WebAccount::new);



    private final AccountFactory factory;

    AccountType(AccountFactory factory) {

        this.factory = factory;

    }



    public Account createAccount(String cf, String iban, int x) {

        return factory.create(cf, iban, x);

    }

}



public interface AccountFactory {

    Account create(String cf, String iban, int x);

}

This solution has a huge advantage over the switch or map approach: if you ever add a new subclass and a new type of account, there is no way you can forget to handle this new type. The other solutions will fail, or give incorrect results, at runtime (instead of compile time)

I would advise against using any sort of type map as it just increases complexity and runtime cost. A switch statement seems the easiest.
Here is an example:

public boolean addAccount(String cf, AccountType type) {

    String iban = name + cf;

    if (bankAccounts.containsKey(iban)) return false;

    Account account;

    switch(type) {

      case CHECKINGACCOUNT:

        account = new CheckingAccount(cf, iban, 0);

        break;

      case DEPOSIT:

        account = new DepositAccount(cf, iban, 0);

        break;

      case WEB:

        account = new WebAccount(cf, iban, 0);

        break;

      default:

         return false; 

   } 

   bankAccounts.put(iban, account);

   return true;

}

However, if you are determined to use a type map:

static Map<AccountType, Class> TYPE_MAP = new HashMap<>() 

{

   {AccountType.CHECKINGACCOUNT, CheckingAccount.class},

   {AccountType.DEPOSIT, DepositAccount.class},

   {AccountType.WEB, WebAccount.class}

};



public boolean addAccount(String cf, AccountType type) {

    String iban = name + cf;

    if (bankAccounts.containsKey(iban)) return false;

    if (!TYPE_MAP.containsKey(type)) return false;

    Class classType = TYPE_MAP.get(type);

    Account account = (Account)classType

      .getDeclaredConstructor(

        String.class,

        String.class,

        Integer.class)

      .newInstance(cf, iban, 0);

    bankAccounts.put(iban, account);

    return true;

}

My opinion is that conceptually everything is fine with this method. I would change it a bit though:

   public boolean addAccount(String cf, AccountType type) {

    String iban = name + cf;

    if (bankAccounts.containsKey(iban)) return false;



    Account ac;

    if (type == CHECKING) {

        ac = new CheckingAccount(cf, iban, 0);

    } else if (type == DEPOSIT) {

        ac = new DepositAccount(cf, iban, 0);

    } else if (type == WEB) {

        ac = new WebAccount(cf, iban, 0);

    }

    bankAccounts.put(iban, cc);

    return true;

}

I do not see any reasons to make it more complex using reflection or factories to create acount instances. It will not give you any benefit, you will just get more code and new person will spend more time to understand it.