Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

##Exceptions:

Exceptions:

It is common practice to throw an IllegalArgumentException if the input values to a method are not legal. In the sepcific case of null values, there is debate about whether the right response is an IllegalArgumentException, or a NullPointerException. I prefer IllegalArgumentException....

Also, in Java, (and most languages), it is very easy to introduce bugs when making small changes to 1-liner conditionals (just ask apple). When making the changes correctly to 1-liners (without introducing bugs) the actual small changes require adding in braces, so small changes change a lot. In general, use braces, even for 1-liners.

The code:

 if(str1 == null || str2 == null)
 return "Null values are not accepted.";

Should be:

 if(str1 == null || str2 == null) {
 throw new IllegalArgumentException("Null values are not accepted.");
 }

One of the reasons for the exception, by the way, is... consider the following code:

System.out.println(multiplyHex("A", multiplyHex("B", null));

With your code in the current state, the result will be:

 "Wrong chracter"

Additionally, the "wrong character" message is .... useless. You shoudl have an exception that indicates what the wrong character is:

int hxval1 = getInt(hex1[j]);
if (hxval1 < 0) {
 throw new IllegalArgumentException(String.format("Illegal character '%s' at position %d in the input value: %s", hex1[j], j, str1));
}

Use exceptions

##Exceptions:

It is common practice to throw an IllegalArgumentException if the input values to a method are not legal. In the sepcific case of null values, there is debate about whether the right response is an IllegalArgumentException, or a NullPointerException. I prefer IllegalArgumentException....

Also, in Java, (and most languages), it is very easy to introduce bugs when making small changes to 1-liner conditionals (just ask apple). When making the changes correctly to 1-liners (without introducing bugs) the actual small changes require adding in braces, so small changes change a lot. In general, use braces, even for 1-liners.

The code:

 if(str1 == null || str2 == null)
 return "Null values are not accepted.";

Should be:

 if(str1 == null || str2 == null) {
 throw new IllegalArgumentException("Null values are not accepted.");
 }

One of the reasons for the exception, by the way, is... consider the following code:

System.out.println(multiplyHex("A", multiplyHex("B", null));

With your code in the current state, the result will be:

 "Wrong chracter"

Additionally, the "wrong character" message is .... useless. You shoudl have an exception that indicates what the wrong character is:

int hxval1 = getInt(hex1[j]);
if (hxval1 < 0) {
 throw new IllegalArgumentException(String.format("Illegal character '%s' at position %d in the input value: %s", hex1[j], j, str1));
}

Use exceptions

Exceptions:

It is common practice to throw an IllegalArgumentException if the input values to a method are not legal. In the sepcific case of null values, there is debate about whether the right response is an IllegalArgumentException, or a NullPointerException. I prefer IllegalArgumentException....

Also, in Java, (and most languages), it is very easy to introduce bugs when making small changes to 1-liner conditionals (just ask apple). When making the changes correctly to 1-liners (without introducing bugs) the actual small changes require adding in braces, so small changes change a lot. In general, use braces, even for 1-liners.

The code:

 if(str1 == null || str2 == null)
 return "Null values are not accepted.";

Should be:

 if(str1 == null || str2 == null) {
 throw new IllegalArgumentException("Null values are not accepted.");
 }

One of the reasons for the exception, by the way, is... consider the following code:

System.out.println(multiplyHex("A", multiplyHex("B", null));

With your code in the current state, the result will be:

 "Wrong chracter"

Additionally, the "wrong character" message is .... useless. You shoudl have an exception that indicates what the wrong character is:

int hxval1 = getInt(hex1[j]);
if (hxval1 < 0) {
 throw new IllegalArgumentException(String.format("Illegal character '%s' at position %d in the input value: %s", hex1[j], j, str1));
}

Use exceptions

added 348 characters in body
Source Link
rolfl
  • 98.1k
  • 17
  • 219
  • 419

##Exceptions:

It is common practice to throw an IllegalArgumentException if the input values to a method are not legal. In the sepcific case of null values, there is debate about whether the right response is an IllegalArgumentException, or a NullPointerException. I prefer IllegalArgumentException....

Also, in Java, (and most languages), it is very easy to introduce bugs when making small changes to 1-liner conditionals (just ask apple). When making the changes correctly to 1-liners (without introducing bugs) the actual small changes require adding in braces, so small changes change a lot. In general, use braces, even for 1-liners.

The code:

 if(str1 == null || str2 == null)
 return "Null values are not accepted.";

Should be:

 if(str1 == null || str2 == null) {
 throw new IllegalArgumentException("Null values are not accepted.");
 }

One of the reasons for the exception, by the way, is... consider the following code:

System.out.println(multiplyHex("A", multiplyHex("B", null));

With your code in the current state, the result will be:

 "Wrong chracter"

Additionally, the "wrong character" message is .... useless. You shoudl have an exception that indicates what the wrong character is:

int hxval1 = getInt(hex1[j]);
if (hxval1 < 0) {
 throw new IllegalArgumentException(String.format("Illegal character '%s' at position %d in the input value: %s", hex1[j], j, str1));
}

Use exceptions

##Exceptions:

It is common practice to throw an IllegalArgumentException if the input values to a method are not legal. In the sepcific case of null values, there is debate about whether the right response is an IllegalArgumentException, or a NullPointerException. I prefer IllegalArgumentException....

Also, in Java, (and most languages), it is very easy to introduce bugs when making small changes to 1-liner conditionals (just ask apple). When making the changes correctly to 1-liners (without introducing bugs) the actual small changes require adding in braces, so small changes change a lot. In general, use braces, even for 1-liners.

The code:

 if(str1 == null || str2 == null)
 return "Null values are not accepted.";

Should be:

 if(str1 == null || str2 == null) {
 throw new IllegalArgumentException("Null values are not accepted.");
 }

One of the reasons for the exception, by the way, is... consider the following code:

System.out.println(multiplyHex("A", multiplyHex("B", null));

With your code in the current state, the result will be:

 "Wrong chracter"

Use exceptions

##Exceptions:

It is common practice to throw an IllegalArgumentException if the input values to a method are not legal. In the sepcific case of null values, there is debate about whether the right response is an IllegalArgumentException, or a NullPointerException. I prefer IllegalArgumentException....

Also, in Java, (and most languages), it is very easy to introduce bugs when making small changes to 1-liner conditionals (just ask apple). When making the changes correctly to 1-liners (without introducing bugs) the actual small changes require adding in braces, so small changes change a lot. In general, use braces, even for 1-liners.

The code:

 if(str1 == null || str2 == null)
 return "Null values are not accepted.";

Should be:

 if(str1 == null || str2 == null) {
 throw new IllegalArgumentException("Null values are not accepted.");
 }

One of the reasons for the exception, by the way, is... consider the following code:

System.out.println(multiplyHex("A", multiplyHex("B", null));

With your code in the current state, the result will be:

 "Wrong chracter"

Additionally, the "wrong character" message is .... useless. You shoudl have an exception that indicates what the wrong character is:

int hxval1 = getInt(hex1[j]);
if (hxval1 < 0) {
 throw new IllegalArgumentException(String.format("Illegal character '%s' at position %d in the input value: %s", hex1[j], j, str1));
}

Use exceptions

Source Link
rolfl
  • 98.1k
  • 17
  • 219
  • 419

##Exceptions:

It is common practice to throw an IllegalArgumentException if the input values to a method are not legal. In the sepcific case of null values, there is debate about whether the right response is an IllegalArgumentException, or a NullPointerException. I prefer IllegalArgumentException....

Also, in Java, (and most languages), it is very easy to introduce bugs when making small changes to 1-liner conditionals (just ask apple). When making the changes correctly to 1-liners (without introducing bugs) the actual small changes require adding in braces, so small changes change a lot. In general, use braces, even for 1-liners.

The code:

 if(str1 == null || str2 == null)
 return "Null values are not accepted.";

Should be:

 if(str1 == null || str2 == null) {
 throw new IllegalArgumentException("Null values are not accepted.");
 }

One of the reasons for the exception, by the way, is... consider the following code:

System.out.println(multiplyHex("A", multiplyHex("B", null));

With your code in the current state, the result will be:

 "Wrong chracter"

Use exceptions

lang-java

AltStyle によって変換されたページ (->オリジナル) /