Skip to content

Conversation

@vshcherbyna-epam
Copy link

No description provided.

case PREPAID_CARD:
case BANK_ACCOUNT:
case WIRE_ACCOUNT:
if (!mSecondLineStrategies.containsKey(TransferMethodSecondLine.class)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need to check if it contains because maps will already do that for you when you call Map.put and I think put is good enough for this operation

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can pre-initialize it no need to assemble it as it goes, like so

Map<String, String> doubleBraceMap  = new HashMap<String, String>() {{
    put("key1", "value1");
    put("key2", "value2");
}};

//noinspection ConstantConditions
return mSecondLineStrategies.get(TransferMethodSecondLine.class);
case PAYPAL_ACCOUNT:
if (!mSecondLineStrategies.containsKey(PayPalAccountTransferMethodSecondLine.class)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need to check if it contains because maps will already do that for you when you call Map.put and I think put is good enough for this operation

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right and in case of Map.put we should also need to verify the previous value.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can pre-initialize it no need to assemble it as it goes, like so

Map<String, String> doubleBraceMap  = new HashMap<String, String>() {{
    put("key1", "value1");
    put("key2", "value2");
}};

Copy link
Collaborator

@fmattos-hw fmattos-hw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vshcherbyna-epam I really appreciate the effort and the robustness of the strategy pattern you introduced but I'd like to use a more simplistic approach. Creating an abstraction and managing instances with the map overcomplicate things and adds memory footprint in the app
Please expose a method in TransferMethodUtil class named getTransferMethodDetail(String): String use a switch case and return the correct value based on each transfer method type and empty in the default case

@vshcherbyna-epam
Copy link
Author

@fmattos-hw, @peter-joseph, @azakrevska-epam
Sorry, I have obstacles to merge it onto the /development and back onto /origing, so Im going to create the new PR here: #44.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants