debugjavaMinor
String find and replace method optimization
Viewed 0 times
methodreplaceoptimizationfindandstring
Problem
I am trying to find a specific header string from different Maps (
```
public class ErrorMessageConverter {
private static Map LEDES1998Bheaders = new HashMap<>();
private static Map LEDES98BIheaders = new HashMap<>();
private static Map LEDES98BI_V2headers = new HashMap<>();
static {
LEDES1998Bheaders.put("(\\binv_date\\b|\\bINVOICE DATE\\b)", "INVOICE DATE");
LEDES1998Bheaders.put("\\binv_id\\b", "INVOICE NUMBER");
LEDES1998Bheaders.put("\\bcl_id\\b", "CLIENT ID");
LEDES1998Bheaders.put("\\blf_matter_id\\b", "LAW FIRM MATTER ID");
LEDES1998Bheaders.put("\\bUNITS\\b", "LINE ITEM NUMBER OF UNITS");
LEDES1998Bheaders.put("(\\bBaseRate\\b|\\bRate\\b|\\btk_rate\\b)", "LINE ITEM UNIT COST");
LEDES98BIheaders.put("\\blf_address/address_info/address_1\\b", "LAW FIRM ADDRESS 1");
LEDES98BIheaders.put("\\blf_address/address_info/city\\b", "LAW FIRM CITY");
LEDES98BIheaders.put("\\btax_rate\\b", "LINE ITEM TAX RATE");
LEDES98BIheaders.put("\\btax_on_charge\\b", "LINE ITEM TAX TOTAL");
LEDES98BIheaders.put("\\btax_type\\b", "LINE ITEM TAX TYPE");
LEDES98BI_V2headers.put("\\binv_reported_tax_total\\b", "INVOICE REPORTED TAX TOTAL");
LEDES98BI_V2headers.put("\\binv_reported_tax_currency\\b", "INVOICE TAX CURRENCY");
}
public static String toUserFriendlyErrorMessage(String parcelType, String message) {
if (parcelType.equals("LEDES1998B")) {
return updateErrorMessage(message, LEDES1998Bheaders);
}
else if (parcelType.equals("LEDES98BI")) {
message = updateErrorMessage(message, LEDES1998Bheaders);
return updateErrorMessage(message, LEDES98BIheaders);
}
else if (parcelType.equals("LEDES98BI V2")
LEDES1998Bheaders, LEDES98BIheaders and LEDES98BI_V2headers) in an errorMessage depends on the parcel type and if the errorMessage has the specific header string I need to replace it with the corresponding value.```
public class ErrorMessageConverter {
private static Map LEDES1998Bheaders = new HashMap<>();
private static Map LEDES98BIheaders = new HashMap<>();
private static Map LEDES98BI_V2headers = new HashMap<>();
static {
LEDES1998Bheaders.put("(\\binv_date\\b|\\bINVOICE DATE\\b)", "INVOICE DATE");
LEDES1998Bheaders.put("\\binv_id\\b", "INVOICE NUMBER");
LEDES1998Bheaders.put("\\bcl_id\\b", "CLIENT ID");
LEDES1998Bheaders.put("\\blf_matter_id\\b", "LAW FIRM MATTER ID");
LEDES1998Bheaders.put("\\bUNITS\\b", "LINE ITEM NUMBER OF UNITS");
LEDES1998Bheaders.put("(\\bBaseRate\\b|\\bRate\\b|\\btk_rate\\b)", "LINE ITEM UNIT COST");
LEDES98BIheaders.put("\\blf_address/address_info/address_1\\b", "LAW FIRM ADDRESS 1");
LEDES98BIheaders.put("\\blf_address/address_info/city\\b", "LAW FIRM CITY");
LEDES98BIheaders.put("\\btax_rate\\b", "LINE ITEM TAX RATE");
LEDES98BIheaders.put("\\btax_on_charge\\b", "LINE ITEM TAX TOTAL");
LEDES98BIheaders.put("\\btax_type\\b", "LINE ITEM TAX TYPE");
LEDES98BI_V2headers.put("\\binv_reported_tax_total\\b", "INVOICE REPORTED TAX TOTAL");
LEDES98BI_V2headers.put("\\binv_reported_tax_currency\\b", "INVOICE TAX CURRENCY");
}
public static String toUserFriendlyErrorMessage(String parcelType, String message) {
if (parcelType.equals("LEDES1998B")) {
return updateErrorMessage(message, LEDES1998Bheaders);
}
else if (parcelType.equals("LEDES98BI")) {
message = updateErrorMessage(message, LEDES1998Bheaders);
return updateErrorMessage(message, LEDES98BIheaders);
}
else if (parcelType.equals("LEDES98BI V2")
Solution
Your code looks fairly effective, but it could be a bunch more efficient if you rearrange the code a bit, do some pre-processing, and reduce your run-time checks. Let's go through that in order....
I recommend a 'set up' stage to your code. Consider an ideal situation at runtime, the code would look something like:
That would be a neat solution, where you have a bunch of rules that you know apply to the messages of a particular type, and that you can then just apply whichever ones are appropriate. That would be the most efficient at use-time too.
How would you implement that? Using some pre-processing, of course, and a new
This preprocessing is key, and the features you should look for here are:
Your rules are convenient in the sense that the
How would I recommend preprocessing things?
So, that rule class is simple enough... how to add them together?
Now, your last part is the
Now your code should go smoothly.
I recommend a 'set up' stage to your code. Consider an ideal situation at runtime, the code would look something like:
public static String toUserFriendlyErrorMessage(String parcelType, String message) {
for (Rule rule : getRules(parcelType)) {
message = rule.process(message);
}
return message;
}That would be a neat solution, where you have a bunch of rules that you know apply to the messages of a particular type, and that you can then just apply whichever ones are appropriate. That would be the most efficient at use-time too.
How would you implement that? Using some pre-processing, of course, and a new
Rule class.This preprocessing is key, and the features you should look for here are:
- the rules are sorted at pre-process time.
- some rules are in multiple sets of operations.
Patterninstances are compiled just once, not at use-time.
Your rules are convenient in the sense that the
parcelType values seems to have an 'extension' type system:LEDES1998Bis rulesLEDES1998Bheaders
LEDES98BIis the same asLEDES1998Bplus the rules inLEDES98BIheaders
LEDES98BI V2is the same asLEDES98BI(and thusLEDES1998B) but also includesLEDES98BI_V2headers
How would I recommend preprocessing things?
private static final class Rule {
private final Pattern key;
private final String replacement;
Rule(String search, String rep) {
key = Pattern.compile(search);
replacement = Matcher.quoteReplacement(rep);
}
String process(String msg) {
return key.matcher(msg).replaceAll(replacement);
}
}So, that rule class is simple enough... how to add them together?
private static final Map RULE_MAP = buildMap();
private static final Map buildMap() {
Map result = new HashMap<>();
List rules = new ArrayList<>();
addRules(rules, LEDES1998Bheaders);
result.put("LEDES1998B", rules.toArray(new Rule[rules.size()]));
addRules(rules, LEDES98BIheaders);
result.put("LEDES98BI", rules.toArray(new Rule[rules.size()]));
addRules(rules, LEDES98BI_V2headers);
result.put("LEDES98BI V2", rules.toArray(new Rule[rules.size()]));
return result;
}
private static final void addRules(List target, Map sources) {
for (Map.Entry me : sources.entrySet()) {
target.add(new Rule(me.getKey(), me.getValue()));
}
}Now, your last part is the
getRules method:private static final Rule[] getRules(String parcelType) {
Rule[] rules = RULE_MAP.get(parcelType);
if (rules == null) {
throw new IllegalStateException("No rules set for type " + parcelType);
}
return rules;
}Now your code should go smoothly.
Code Snippets
public static String toUserFriendlyErrorMessage(String parcelType, String message) {
for (Rule rule : getRules(parcelType)) {
message = rule.process(message);
}
return message;
}private static final class Rule {
private final Pattern key;
private final String replacement;
Rule(String search, String rep) {
key = Pattern.compile(search);
replacement = Matcher.quoteReplacement(rep);
}
String process(String msg) {
return key.matcher(msg).replaceAll(replacement);
}
}private static final Map<String, Rule[]> RULE_MAP = buildMap();
private static final Map<String, Rule[]> buildMap() {
Map<String, Rule[]> result = new HashMap<>();
List<Rule> rules = new ArrayList<>();
addRules(rules, LEDES1998Bheaders);
result.put("LEDES1998B", rules.toArray(new Rule[rules.size()]));
addRules(rules, LEDES98BIheaders);
result.put("LEDES98BI", rules.toArray(new Rule[rules.size()]));
addRules(rules, LEDES98BI_V2headers);
result.put("LEDES98BI V2", rules.toArray(new Rule[rules.size()]));
return result;
}
private static final void addRules(List<Rule> target, Map<String,String> sources) {
for (Map.Entry<String,String> me : sources.entrySet()) {
target.add(new Rule(me.getKey(), me.getValue()));
}
}private static final Rule[] getRules(String parcelType) {
Rule[] rules = RULE_MAP.get(parcelType);
if (rules == null) {
throw new IllegalStateException("No rules set for type " + parcelType);
}
return rules;
}Context
StackExchange Code Review Q#86554, answer score: 3
Revisions (0)
No revisions yet.