patternjavaMinor
Dynamic condition on string in method
Viewed 0 times
stringconditionmethoddynamic
Problem
I have the following method:
In the above method I have to loop over something similar to tree structure bottom up and check if the name of the node EQUALS to entry name in the map. if yes - I will calculate the price according to it.
As a second choice, I need to loop again to check if the name CONTAINS the entry name in the map.
I wrote a similar method:
As you see, the only difference is the name.equals vs name.contains in the 4th line.
I don't like this duplicated
Float getPriceEqualsCategoryName(BrowseNode node, Float productPrice) {
String name = node.getName();
for (Entry> entry : categoryToFee.entrySet()) {
if (name.equals(entry.getKey())) {
List values = entry.getValue();
for (RangeValue value : values) {
if (productPrice < value.getMinValue())
return value.getMinValue();
if (value.getRange().contains(productPrice))
return value.getReferPercentFee() / 100 * productPrice;
}
}
}
if (node.getAncestors() == null)
return 0f;
return getPriceEqualsCategoryName(node.getAncestors().getBrowseNode().get(0), productPrice);
}In the above method I have to loop over something similar to tree structure bottom up and check if the name of the node EQUALS to entry name in the map. if yes - I will calculate the price according to it.
As a second choice, I need to loop again to check if the name CONTAINS the entry name in the map.
I wrote a similar method:
Float getPriceContainsCategoryName(BrowseNode node, Float productPrice) {
String name = node.getName();
for (Entry> entry : categoryToFee.entrySet()) {
if (name.contains(entry.getKey())) {
List values = entry.getValue();
for (RangeValue value : values) {
if (productPrice < value.getMinValue())
return value.getMinValue();
if (value.getRange().contains(productPrice))
return value.getReferPercentFee() / 100 * productPrice;
}
}
}
if (node.getAncestors() == null)
return 0f;
return getPriceContainsCategoryName(node.getAncestors().getBrowseNode().get(0), productPrice);
}As you see, the only difference is the name.equals vs name.contains in the 4th line.
I don't like this duplicated
Solution
Extract Duplication
You could extract parts of the duplication to their own method:
And use it like this:
Use Interface
You could also use an interface:
Restructure
It seems that your method currently does two things: It finds a category by name, and it calculate/finds some kind of price for a category. I would write two completely separate methods for each tasks, and then try to reduce duplication.
You could extract parts of the duplication to their own method:
private Float someResonableName(Float productPrice, List values) {
for (RangeValue value : values) {
if (productPrice < value.getMinValue()) {
return value.getMinValue();
}
if (value.getRange().contains(productPrice)) {
return value.getReferPercentFee() / 100 * productPrice;
}
}
return -1;
}And use it like this:
if (name.contains(entry.getKey())) {
Float result = someResonableName(productPrice, entry.getValue());
if (result != -1) {
return result;
}
}Use Interface
You could also use an interface:
interface NameKeyRelation {
boolean isRelation(String name, RangeValue key);
}
Float getPriceContainsCategoryName(BrowseNode node, Float productPrice) {
return getPriceContainsCategoryName(node, productPrice, (String name, RangeValue key) -> name.contains(key)));
}
Float getPriceEqualsCategoryName(BrowseNode node, Float productPrice) {
return getPriceContainsCategoryName(node, productPrice, (String name, RangeValue key) -> name.equals(key)));
}
Float getPriceCategoryName(BrowseNode node, Float productPrice, NameKeyRelation relation) {
String name = node.getName();
for (Entry> entry : categoryToFee.entrySet()) {
if (relation.isRelation(name, entry.getKey())) {
Float result = someResonableName(productPrice, entry.getValue());
if (result != -1) {
return result;
}
}
}
if (node.getAncestors() == null)
return 0f;
return getPriceCategoryName(node.getAncestors().getBrowseNode().get(0), productPrice);
}Restructure
It seems that your method currently does two things: It finds a category by name, and it calculate/finds some kind of price for a category. I would write two completely separate methods for each tasks, and then try to reduce duplication.
Code Snippets
private Float someResonableName(Float productPrice, List<RangeValue> values) {
for (RangeValue value : values) {
if (productPrice < value.getMinValue()) {
return value.getMinValue();
}
if (value.getRange().contains(productPrice)) {
return value.getReferPercentFee() / 100 * productPrice;
}
}
return -1;
}if (name.contains(entry.getKey())) {
Float result = someResonableName(productPrice, entry.getValue());
if (result != -1) {
return result;
}
}interface NameKeyRelation {
boolean isRelation(String name, RangeValue key);
}
Float getPriceContainsCategoryName(BrowseNode node, Float productPrice) {
return getPriceContainsCategoryName(node, productPrice, (String name, RangeValue key) -> name.contains(key)));
}
Float getPriceEqualsCategoryName(BrowseNode node, Float productPrice) {
return getPriceContainsCategoryName(node, productPrice, (String name, RangeValue key) -> name.equals(key)));
}
Float getPriceCategoryName(BrowseNode node, Float productPrice, NameKeyRelation relation) {
String name = node.getName();
for (Entry<String, List<RangeValue>> entry : categoryToFee.entrySet()) {
if (relation.isRelation(name, entry.getKey())) {
Float result = someResonableName(productPrice, entry.getValue());
if (result != -1) {
return result;
}
}
}
if (node.getAncestors() == null)
return 0f;
return getPriceCategoryName(node.getAncestors().getBrowseNode().get(0), productPrice);
}Context
StackExchange Code Review Q#80143, answer score: 4
Revisions (0)
No revisions yet.