HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMinor

String find and replace method optimization

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
methodreplaceoptimizationfindandstring

Problem

I am trying to find a specific header string from different Maps (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:

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.



  • Pattern instances 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:

  • LEDES1998B is rules LEDES1998Bheaders



  • LEDES98BI is the same as LEDES1998B plus the rules in LEDES98BIheaders



  • LEDES98BI V2 is the same as LEDES98BI (and thus LEDES1998B) but also includes LEDES98BI_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.