Parsing a complex number using regular expressions

Question

Requirements were to create a constructor for a complex number, that receives a string. For example: \$3+5i\$. The regex extracts positive or negative numbers from the string with optional decimal.

My professor told me that my regex was "beyond wrong":

public Complex(String str) {


        ArrayList<Double> list = new 
            ArrayList<Double>();

        Pattern p = Pattern.compile("[-+]?[0-9]*\\.?[0-9]");
        // toString() handles the letter i

        // find positive and negative doubles in string and add to list
        Matcher m = p.matcher(str);
        while (m.find()) {
            double num = Double.parseDouble(m.group());
            list.add(num);
        }
        this.re = list.get(0);
        this.im = list.get(1);
}

Here is a link to the class if you need a better understanding of the code.

He then posted his chosen correct solution, which I found to be repetitive. He may argue that I am not checking for i, but I see no point of storing i if we are not using it later.

In addition, if he argues that I am not checking for and storing i, that would mean that if I did store them, I would explicitly have to go exclude the i from the string when I perform mathematical operations on the numbers. It seems counter-productive.

Here is the solution he preferred:

public Complex(String c) {

      String numberNoWhiteSpace = c.replaceAll("\\s","");

      // Matches complex number with BOTH real AND imaginary parts.  
      // Ex: -3-2.0i
      Pattern patternA = Pattern.compile("([-]?[0-9]+\\.?[0-9]?)([-|+]+[0-9]+\\.?[0-9]?)[i$]+");

      // Matches ONLY real number.
      // Ex: 3.145
      Pattern patternB = Pattern.compile("([-]?[0-9]+\\.?[0-9]?)$");

      // Matches ONLY imaginary number.
      // Ex: -10i
      Pattern patternC = Pattern.compile("([-]?[0-9]+\\.?[0-9]?)[i$]");

      Matcher matcherA = patternA.matcher(numberNoWhiteSpace);
      Matcher matcherB = patternB.matcher(numberNoWhiteSpace);
      Matcher matcherC = patternC.matcher(numberNoWhiteSpace);

      if (matcherA.find()) {
          real = Double.parseDouble(matcherA.group(1));
          imaginary = Double.parseDouble(matcherA.group(2));
      } else if (matcherB.find()) {
          real = Double.parseDouble(matcherB.group(1));
          imaginary = 0;
      } else if (matcherC.find()) {
          real = 0;
          imaginary = Double.parseDouble(matcherC.group(1));
      }   
}

Answer 1

Both of your solutions are wrong, in different ways. Yours is worse, though, because it fails in a way that can actually succeed in giving completely wrong answers.

Your solution

Your technique is, as your professor noted, sloppy. It falls apart in several cases that I would consider reasonable inputs:

• Pure imaginary numbers such as "5i"
• No imaginary component, such as "1"
• No imaginary coefficient, such as "5+i"
• Imaginary number with a space, such as "3 - 3i"
• Decimals more precise than one-tenth, such as "3.14"

Furthermore, using an ArrayList seems extravagant. You shouldn't need one.

Professor's solution¹

Sanitizing the input by discarding spaces — is that a good idea? Why? Is there another way to handle spaces?

Why three Patterns, three Matchers, and three .find()s? Can it be done with fewer?

Within each pattern, is your regex more correct, or his? Can you spot any bugs in his character classes (i.e. the stuff inside [square brackets])?

Are there any inputs that also fail with his code? Does his code accept any inputs that should be considered invalid?

---

¹ We shouldn't really be criticizing code that you didn't write, so I'll give you some questions to ponder instead.

Answer 2

        ArrayList<Double> list = new 
            ArrayList<Double>();

I'm not sure why this is on two lines.

Also, if you are on the latest Java, you don't need the second Double. Let the compiler figure it out for you. This also makes it easier to change to a different type later (e.g. BigDecimal).

    List<Double> list = new ArrayList<>();

Finally, you usually don't define variables as an implementation type but as the interface. This makes it much easier if you choose to change implementations in the future.

Note: I agree with the previous comment that you don't need a collection here. Consider the following code:

        while (m.find()) {
            double num = Double.parseDouble(m.group());
            list.add(num);
        }
        this.re = list.get(0);
        this.im = list.get(1);

Why not just say

        if (m.find()) {
            real = Double.parseDouble(m.group());
            imaginary = m.find() ? Double.parseDouble(m.group()) : 0.0;
        }

You replace six lines of code with four, although one is more complicated than it had been. You save use of the List entirely. You handle something with a real part but no imaginary part better. It still doesn't handle numbers with only an imaginary part or invalid input with two imaginary parts. Nor invalid input with three or more parts. But while you processed more parts, you were throwing away everything after the first two.

Note that I changed your variable names to be longer and more descriptive. I also removed the unnecessary this.. General practice in java is to leave it off unless needed to disambiguate with a method parameter. E.g. this.name = name; is one of the few times that you'd see this. used with an object field.

Answer 3

I Prefer the second one. But there's still some suggestions for the second one:

1. patternA/B/C should use better self-explanation names.
2. PatternA/B/C should extract to static final fields.

BTY: what is the purpose of this code snappet? Used in real project? For regex excise? For coding excise?