CODEWITHSUNDEEP
javacode-duplication
amit
amit

Reputation: 178411

avoid code duplication

consider the following code:

if (matcher1.find()) {
   String str = line.substring(matcher1.start()+7,matcher1.end()-1);
   /*+7 and -1 indicate the prefix and suffix of the matcher... */    
   method1(str);
}
if (matcher2.find()) {
   String str = line.substring(matcher2.start()+8,matcher2.end()-1);
   method2(str);
}
...

I have n matchers, all matchers are independent (if one is true, it says nothing about the others...), for each matcher which is true - I am invoking a different method on the content it matched.
question: I do not like the code duplication nor the "magic numbers" in here, but I'm wondering if there is better way to do it...? (maybe Visitor Pattern?) any suggestions?

Upvotes: 2

Views: 359

Answers (4)

Alois Cochard
Alois Cochard

Reputation: 9862

Create an abstract class, and add offset in subclass (with string processing too... depending of your requirement).

Then populate them in a list and process the list.

Here is a sample absract processor:

public abstract class AbsractProcessor {

    public void find(Pattern pattern, String line) {
        Matcher matcher = p.matcher(line);
        if (matcher.find()) {
            process(line.substring(matcher.start() + getStartOffset(), matcher.end() - getEndOffset()));
        }
    }

    protected abstract int getStartOffset();

    protected abstract int getEndOffset();

    protected abstract void process(String str);

}

Upvotes: 4

Joachim Sauer
Joachim Sauer

Reputation: 308001

Simple mark the part of the regex that you want to pass to the method with a capturing group.

For example if your regex is foo.*bar and you are not interested in foo or bar, make the regex foo(.*)bar. Then always grab the group 1 from the Matcher.

Your code would then look like this:

method1(matcher1.group(1));
method2(matcher2.group(2));
...

One further step would be to replace your methods with classes implementing an like this:

public interface MatchingMethod {
  String getRegex();
  void apply(String result);
}

Then you can easily automate the task:

for (MatchingMethod mm : getAllMatchingMethods()) {
  Pattern p = Pattern.compile(mm.getRegex());
  Matcher m = p.matcher(input);
  while (m.find()) {
    mm.apply(m.group(1));
}

Note that if performance is important, then pre-compiling the Pattern can improve runtime if you apply this to many inputs.

Upvotes: 1

duedl0r
duedl0r

Reputation: 9414

use Cochard's solution combined with a factory (switch statement) with all the methodX methods. so you can call it like this:

Factory.CallMethodX(myEnum.MethodX, str)

you can assign the myEnum.MethodX in the population step of Cochard's solution

Upvotes: 0

anon
anon

Reputation:

You could make it a little bit shorter, but I the question is, is this really worth the effort:

private String getStringFromMatcher(Matcher matcher, int magicNumber) {
   return line.subString(matcher.start() + magicNumber, matcher.end() - 1 )
}

if (matcher1.find()) {
method1(getStringFromMatcher(matcher1, 7);
}

if (matcher2.find()) {
method2.(getStringFromMatcher(mather2, 8);
}

Upvotes: 0

Related Questions