I've to create a class MyCalculator which consists of a single method long power(int, int). This method takes two integers, and, as parameters and finds Math.pow(n,p). - If either n or p is negative, then the method must throw an exception which says "n or p should not be negative.". - If both n and p are zero, then the method must throw an exception which says "n and p should not be zero."
class MyCalculator {
/*
* Create the method long power(int, int) here.
*/
long power(int n, int p) throws Exception {
if(n==0 && p==0) {
throw new Exception("n and p should not be zero.");
} else if(n<0 || p<0) {
throw new Exception("n or p should not be negative.");
} else {
return (long)Math.pow(n, p);
}
}
}
Is there any way to use Exception handling in a better way in such questions? We have to perform this check in most of the questions. So I need to understand it clearly.
5 Answers 5
Is there any way to use Exception handling in a better way in such questions?
What you have is a very good start.
I would suggest refining your exception type where possible. In your case, I would suggest using IllegalArgumentException. Using a specific subtype helps with diagnosing/debugging.
Additionally, since IllegalArgumentException is a type of RuntimeException, you can then remove the throws Exception
from your method signature.
long power(int n, int p) {
if (n == 0 && p == 0) {
throw new IllegalArgumentException("n and p should not be zero.");
} else if (n < 0 || p < 0) {
throw new IllegalArgumentException("n or p should not be negative.");
}
return (long) Math.pow(n, p);
}
Just a little improvement of in the code extending from JvR's answer: I think it's possible to get rid of the else if since you're throwing exceptions for invalid inputs. I do agree with JvR in using a specific subtypes for exceptions. But I think it's best if you create your own exception extending from IllegalArgumentException like so:
public class InvalidCalculatorInputException extends IllegalArgumentException{
public InvalidCalculatorInputException(String message){
super(message);
}
}
Also, I think it's best if you separate the logic for handling the exceptions from the power method. This is to make the power method more readable. Applying all the things I've mentioned, this is my refactored version of your MyCalculator class:
class MyCalculator {
long power(int base, int exponent) {
checkIfInputsAreValid(base, exponent);
return (long) Math.pow(base, exponent);
}
private void checkIfInputsAreValid(int base, int exponent){
if(base == 0 && exponent == 0)
throw new InvalidCalculatorInputException("The base and the exponent should not be both 0.");
if(base < 0 || exponent < 0)
throw new InvalidCalculatorInputException("Either base or exponent should not be negative. Base is " + base + "; exponent is " + exponent);
}
}
-
\$\begingroup\$ I didn't find the class InvalidArgumentException in the Java API. Did you mean IllegalArgumentException? \$\endgroup\$Ralf Kleberhoff– Ralf Kleberhoff2017年10月12日 19:03:27 +00:00Commented Oct 12, 2017 at 19:03
-
\$\begingroup\$ oh yeah, it's IllegalArgumentException lol. My bad :)) \$\endgroup\$Rocket Pingu– Rocket Pingu2017年10月12日 21:30:58 +00:00Commented Oct 12, 2017 at 21:30
You might consider using custom exceptions coupled with an assertion class. This would give you a lot of good exception type information to catch and work with, instead of just descriptions in a single exception type. As well, it cleans up your method nicely by stating in English what is happening.
public class MyCalculator
{
public long Power(int n, int p)
{
Assert.AreNotZero("n and p", n, p);
Assert.AreNotNegative("n and p", n, p);
return (long)Math.Pow(n, p);
}
}
public static class Assert
{
public static void AreNotZero(string arg, params int[] values)
{
if (values.Where(v => v == 0).Any())
throw new InvalidZeroException(arg);
}
public static void AreNotNegative(string arg, params int[] values)
{
if (values.Where(v => v < 0).Any())
throw new InvalidNegativeException(arg);
}
}
public class InvalidZeroException : Exception
{
public InvalidZeroException(string argument) : base(argument + " should not be zero") { }
}
public class InvalidNegativeException : Exception
{
public InvalidNegativeException(string argument) : base(argument + " should not be negative") { }
}
-
\$\begingroup\$ This is about Java, not C#. :) \$\endgroup\$Roland Illig– Roland Illig2017年10月13日 06:55:00 +00:00Commented Oct 13, 2017 at 6:55
-
\$\begingroup\$ @RolandIllig i believe it's about good exception practices and the information is translatable. \$\endgroup\$TBone– TBone2017年10月13日 11:28:30 +00:00Commented Oct 13, 2017 at 11:28
If the error handling takes too much space in your code, you can invent a little helper function.
private static void ensure(boolean condition, String message) {
if (!condition) {
throw new IllegalArgumentException(message);
}
}
Using this helper method, you can write the power
function like this:
long power(int n, int p) {
ensure(n != 0 || p != 0, "n and p should not be zero.");
ensure(n >= 0, "n should not be negative.");
ensure(p >= 0, "p should not be negative.");
return (long)Math.pow(n, p);
}
Note that the messages are all string literals. If you were to use a string expression like "n is negative: " + n
instead, this string would be computed on every invocation of power
, which would make your code very slow. Using string literals, on the other hand, doesn't cost any time.
Thanks for sharing your code.
for your request of reusing the checks you have 2 options.
Either one requires that you move the checks into a parameterized method first.
Before you can do that you have to understand that the current method is exited immediately when an exception is thrown. This means than you don't need the final else
:
class MyCalculator {
long power(int n, int p) throws Exception {
if(n==0 && p==0) {
throw new Exception("n and p should not be zero.");
} else if(n<0 || p<0) {
throw new Exception("n or p should not be negative.");
}
return (long)Math.pow(n, p);
}
}
Now you can select the complete if/else
and invoke your IDE's extract method refactoring (eg. in eclipse press <1> to bring up the quickfix menu and find the refactoring there).
This will change your code to this:
class MyCalculator {
long power(int n, int p) throws Exception {
verifyOperandsPreconditions(n, p);
return (long)Math.pow(n, p);
}
private void verifyOperandsPreconditions(int n, int p) throws Exception {
if(n==0 && p==0) {
throw new Exception("n and p should not be zero.");
} else if(n<0 || p<0) {
throw new Exception("n or p should not be negative.");
}
}
}
From now on you have 2 was to go:
legacy inheritance approach
In this solution you create a super class with the check method and a subclass that does the actual work:This will change your code to this:
class MyCalculatorBaseClass { // mind the new scope! protected void verifyOperandsPreconditions(int n, int p) throws Exception { if(n==0 && p==0) { throw new Exception("n and p should not be zero."); } else if(n<0 || p<0) { throw new Exception("n or p should not be negative."); } } } class MyCalculator extends MyCalculatorBaseClass { long power(int n, int p) throws Exception { verifyOperandsPreconditions(n, p); // call to base class method return (long)Math.pow(n, p); } }
modern composition approach
In this solution you useMyCalculatorBaseClass
as a dependency in yourMyCalculator
class:class MyCalculatorChecks { // again we change the scope! public void verifyOperandsPreconditions(int n, int p) throws Exception { if(n==0 && p==0) { throw new Exception("n and p should not be zero."); } else if(n<0 || p<0) { throw new Exception("n or p should not be negative."); } } } class MyCalculator { MyCalculatorChecks checks = new MyCalculatorChecks(); long power(int n, int p) throws Exception { checks.verifyOperandsPreconditions(n, p); // call to method on dependency return (long)Math.pow(n, p); } }
About the visibility scopes:
In both example we had to change the visibility scope of the extracted method in order to enable access from other classes.
I choose protected
in the first approach to signal that this method is meant to be used by sub classes.
I choose public
in the second approach to signal that this method is meant to be used by any other class.
Which of the two approaches you mentioned is better? "modern composition approach or legacy inheritance approach" – user2769790
The "modern composition approach" of cause. Thought my wording was clear enough...
And do we really need to create a separate class for a single method? What's a good practice for interview whiteboard preparation? – user2769790
If there is only one method to be shared: Yes.
But most likely you will collect several methods in one object/class.
On the other hand the Single Responsibility Pattern must be applied to that common class too which means that you might sometimes need more than one class for such common behavior.
-
\$\begingroup\$ > Which of the two approaches you mentioned is better? "modern composition approach or legacy inheritance approach" > And do we really need to create a separate class for a single method? What's a good practice for interview whiteboard preparation? \$\endgroup\$user2769790– user27697902017年10月11日 21:31:14 +00:00Commented Oct 11, 2017 at 21:31
-
\$\begingroup\$ (While there is "to exact", that probably should read extracted: never trust a spelling checker to catch all typing errors. And
of cause
still is wrong, of course, as is "the second"tho
.) \$\endgroup\$greybeard– greybeard2017年10月12日 10:56:27 +00:00Commented Oct 12, 2017 at 10:56 -
1\$\begingroup\$ I downvoted your answer because I think it violates readability, which to me is the most important feature of professional code. In both variants you have to check two classes until you see that the calculator throws an exception in specific cases. \$\endgroup\$Ralf Kleberhoff– Ralf Kleberhoff2017年10月12日 18:55:53 +00:00Commented Oct 12, 2017 at 18:55
-
\$\begingroup\$ @RalfKleberhoff could you please explain how this affects readability in a bad way? \$\endgroup\$Timothy Truckle– Timothy Truckle2017年10月12日 19:28:16 +00:00Commented Oct 12, 2017 at 19:28
-
\$\begingroup\$ Having a method called
extracted
at the end of your answer is unacceptable. It's only acceptable for a tiny moment for explaining how to fix up the method name after doing a refactoring like this. \$\endgroup\$Roland Illig– Roland Illig2017年10月12日 21:55:50 +00:00Commented Oct 12, 2017 at 21:55
Math.pow()
. Why should it be an error to raise a negative integer to a positive power? \$\endgroup\$