Question

Please have a look at mine constructor below, I'm creating a Fraction from a String:

public Fraction(String str)
    {
        if(str.isEmpty())
        {
            throw new IllegalArgumentException("The str (String) parameter cannot be empty!");
        }
             int[] fractionData= new int[2];
             String[] data = str.split("/");
             try
             {
                 if(data.length==0)
                 throw new IllegalArgumentException("The str (String) parameter cannot be empty");
             }
             catch (IllegalArgumentException ex)
             {
                 System.out.println(ex.toString());
             }
             try
             {
                fractionData[0] = Integer.parseInt(data[0]); 
             }
             catch (IllegalArgumentException ex)
             {
                 System.out.println(ex.toString());
             }

             try
             {
                 fractionData[1] = Integer.parseInt(data[1]);
                 if(fractionData[1]==0) throw new ArithmeticException("Denominator can't be 0");
             }
             catch (ArithmeticException ex)
             {
                 System.out.println(ex.toString());
             }
             fractionData = normalize(fractionData[0],fractionData[1]);
             this.numerator = fractionData[0];
             this.denominator = fractionData[1];
    }

I'm catching the IllegalArgumentException fine, but fail to catch the ArithemticException. I can test both successfully

@Test(expected=IllegalArgumentException.class)
    public void testIllegalArgumenException() throws IllegalArgumentException
    {
        Fraction g =  new Fraction("");
    }
    @Test(expected=ArithmeticException.class)
    public void testArithmeticException() throws ArithmeticException
    {
        Fraction g = new Fraction(1/0);
    }

Thanks the comments from @xp500 I've changed my code to:

public Fraction(String str)
    {
        if(str.isEmpty()) throw new IllegalArgumentException("The str (String) parameter cannot be empty!");
             int[] fractionData= new int[2];
             String[] data = str.split("/");
             if (data.toString().matches("[a-zA-Z]+")) throw new NumberFormatException("only numbers allowed in the string");
             fractionData[0] = Integer.parseInt(data[0]);
             fractionData[1] = Integer.parseInt(data[1]);
             if(fractionData[1]==0) throw new ArithmeticException("Denominator can't be 0"); 
             fractionData = normalize(fractionData[0],fractionData[1]);
             this.numerator = fractionData[0];
             this.denominator = fractionData[1];
    }

It doesn't quote "only numbers allowed in the string", but stops initializing the Fraction as 0/0 in case String with letters is used to initialize the Fraction and throws the other exceptions with quoted text. The lesson for me is: DON'T CATCH EXCEPTIONS UNLESS YOU'RE ACTUALLY DOING SOMETHING WITH THEM

Was it helpful?

Solution 2

A couple of comments about the code.

Your usage of the exceptions is making the code unreadable and hard to follow. You should try to use less try catch blocks. I think it would be better if you wrote something along the lines of

if (data.length==0) {
   System.out.println(ex.toString());
   throw new IllegalArgumentException("The str (String) parameter cannot be empty");
}

and not catch that exception, since you want to tell the caller that an exception occured.

Also, fractionData[0] = Integer.parseInt(data[0]); throws a NumberFormatException, not IllegalArgumentException

The ArithmeticException isn't being thrown since you are catching it inside the constructor, and not rethrowing it. Please note that, after catching it, your fraction will be initialized in an invalid state, since

fractionData = normalize(fractionData[0],fractionData[1]);
this.numerator = fractionData[0];
this.denominator = fractionData[1];

will be executed. Again, you might want to rewrite those lines for something like

if(fractionData[1]==0) {
    System.out.println(ex.toString());
    throw new ArithmeticException("Denominator can't be 0");
}

You don't need to write throws Exception, in your test methods, since you are expecting an exception to be thrown, the method itself won't throw that exception.

I hope that helps!

OTHER TIPS

Yo are catching ArithmeticException but you are not retrowing it(like other exceptions)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top