6
\$\begingroup\$

Here are 2 functions defined that do the exactly same thing: take input a template (in which one wants to replace some substrings) and array of strings values (key value pair to replace, such as [subStrToReplace1,value1,subStrToReplace1,value2,.....]) and returns the replaced String.

In second function I am iterating over words of the templates and searching for the relevant key if exist in a HashMap and then next word. If I want to replace a word with some substring, which I again want to replace with some other key in values, I need to iterate over template twice.

I would like to know which one should I use and why. Any better alternative than these are also welcome.

1st function

public static String populateTemplate1(String template, String... values) {
        String populatedTemplate = template;
        for (int i = 0; i < values.length; i += 2) {
            populatedTemplate = populatedTemplate.replace(values[i], values[i + 1]);
        }

        return populatedTemplate;    
    }

2nd function

   public static String populateTemplate2(String template, String... values) {

        HashMap<String, String> map = new HashMap<>();
        for (int i = 0; i < values.length; i += 2) {
            map.put(values[i],values[i+1]);
        }      

        StringBuilder regex = new StringBuilder();
        boolean first = true;
        for (String word : map.keySet()) {
            if (first) {
                first = false;
            } else {
                regex.append('|');
            }
            regex.append(Pattern.quote(word));
        }
        Pattern pattern = Pattern.compile(regex.toString());

        int N0OfIterationOverTemplate =2;
        // Pattern allowing to extract only the words
        // Pattern pattern = Pattern.compile("\\w+");
        StringBuilder populatedTemplate=new StringBuilder();;

        String temp_template=template;

        while(N0OfIterationOverTemplate!=0){
        populatedTemplate = new StringBuilder();
        Matcher matcher = pattern.matcher(temp_template);
        int fromIndex = 0;
        while (matcher.find(fromIndex)) {
            // The start index of the current word
            int startIdx = matcher.start();
            if (fromIndex < startIdx) {
                // Add what we have between two words
                populatedTemplate.append(temp_template, fromIndex, startIdx);
            }
            // The current word
            String word = matcher.group();
            // Replace the word by itself or what we have in the map
            // populatedTemplate.append(map.getOrDefault(word, word));

            if (map.get(word) == null) {
                populatedTemplate.append(word);
            }
            else {
                populatedTemplate.append(map.get(word));
            }

            // Start the next find from the end index of the current word
            fromIndex = matcher.end();
        }
        if (fromIndex < temp_template.length()) {
            // Add the remaining sub String
            populatedTemplate.append(temp_template, fromIndex, temp_template.length());
        }

        N0OfIterationOverTemplate--;
        temp_template=populatedTemplate.toString();
    }
        return populatedTemplate.toString();
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Variable naming

N0OfIterationOverTemplate is not only a mouthful, but anyone will also start to wonder about that weird spelling choice of combining a zero 0 with a capital O together. countIterationOverTemplate is much easier to follow.

null handling

Erm, consider how you want to handle potential null values safely.

Biggest difference

Test case: "template", "a", "b", "a", "c"

In the first method, it immediately replaces the "a"s with "b"s, so the second substitution to "c" is effectively a no-op. You end up with "templbte".

In the second method, there's only one substitution being performed, due to how Map.put(K, V) works. Not only that, it's indeterministic which substitution will happen due to how HashMap.keySet() has no predictable iteration order.

Interfaces over implementations declaration

This is the least of your concern here given the biggest difference above, but you are highly recommended to declare map as a Map instead of HashMap because where it's used, it does not need to be known about its actual Map implementation.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.