• Post Reply Bookmark Topic Watch Topic
  • New Topic

Validating a Number  RSS feed

 
Lee Armet
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Why won't this work? I get an error of: This method must return a type of boolean.

public boolean valInteger(String fieldname) {
String sInts = "0123456789";
for (int i = 0; i < fieldname.length(); i++) {
if (fieldname.indexOf(sInts) == -1) {
return false;
} else {
return true;
}
}
}
 
Bear Bibeault
Author and ninkuma
Marshal
Posts: 66305
152
IntelliJ IDE Java jQuery Mac Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Moved to Java in General (beginner)
 
Bear Bibeault
Author and ninkuma
Marshal
Posts: 66305
152
IntelliJ IDE Java jQuery Mac Mac OS X
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
For a method with a non-void return type, every execution path through the method must have a return statement. Carefully inspect your method for the execution paths. Do they make sense? Does every path have a return?
 
Paul Clapham
Sheriff
Posts: 22823
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It's because you don't specify what to return after the for-loop completes. And if fieldname.length() is zero, then the body of the loop will never be executed, so you need to specify what to return in that case.

As the code is written, you don't need the for-loop at all, since you do the exact same test (see if the fieldname variable contains the string "0123456789") every time through the loop. And anyway, if the loop body is executed even once, you're going to return either true or false on the first iteration, so there's no point in looping.

However from the name of the method I guess that isn't the test you meant to make. It looks to me as if you are trying to compare each character of a string to see if it's a digit. If all of them are digits, then you want to return true. If any of them are not digits, then you want to return false.

That means that you want to return false immediately when you see a non-digit. I.e. inside the loop. But you want to return true only after you have looked at all the characters and ensured that they are digits. I.e. after the loop completes.

You also need to fix your "is this a digit" check. First, it should look at each individual character of the string, not at the whole string. Second, seeing if "0123456789" is in something is the reverse of what you really want to test.
 
Lee Armet
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I found a better way.

public boolean validateInt( String field) {
try {
int value = Integer.parseInt( field);
return true;
} catch( Exception e) {
return false;
}
}
 
Jeff Albertson
Ranch Hand
Posts: 1780
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Your original method (with kinks removed) would validate arbitrarily long strings, like "88888888888888888888888888888888888888888888888888". Was that a bug or a feature?
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Using a try-catch block for such a test is generally considered poor form:
1. Exceptions should not be used to control the flow of execution that way
2. you should not have a return statement in a catch block.

Check out the Apache Commons StringUtils.isNumeric() implementation.
 
Jeff Albertson
Ranch Hand
Posts: 1780
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hmmm... The try+Integer.parseInt seems to be the least painful way in the J2SE API to vaildate if a String represents a 32-bit int. (That StringUtil method ignores the magnitude and can't handle negative numbers!)
 
Scott Selikoff
author
Bartender
Posts: 4093
21
Eclipse IDE Flex Google Web Toolkit
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I concur, for numeric checking, I tend to use try/catch blocks... until Sun adds an isInt() method anyway.
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
My point was to discourage the use of exceptions to control normal flow of execution. See these articles:
http://www.onjava.com/pub/a/onjava/2003/11/19/exceptions.html?page=2
http://www.informit.com/articles/article.asp?p=337136&seqNum=3&rl=1

The StringUtils.isNumeric implementation was cited as an example of how you'd write code that didn't use exceptions to control flow. If it doesn't meet your requirements, you are free to add enhancements that do.
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In JDK 5 you can use the hasNextInt() method in Scanner. (Or hasNextLong(), hasNextBigInteger(), etc.) Note that by default this will ignore additional input after the number (if separated by whitespace) but you can circumvent this by changing the delimiter, e.g.:

The "\\Z" is a regular expression which says the only delimiter in the expression is the end of input. This forces the scanner to consider the entire string as a number, not just the beginning of the string.

In JDK 1.4+, it's also possible to construct your own regular expressions for numbers. However for most people it's simpler and more reliable to use Integer.parseInt() and catch the exception. Yes, it's best to avoid using exceptions for normal control flow - but this is one case where, at least prior to JDK 5, the advantages of simplicity and reliability you get from Integer.parseInt() generally outweigh other concerns, IMO.
[ December 14, 2005: Message edited by: Jim Yingst ]
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
BTW, for checking if a String is a number, you could use org.apache.commons.lang.math.NumberUtils.isNumber(). Note that it doesn't use exceptions either
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you do use the parseInt() solution, I would recommend restricting the catch block to only the specific exception which you expect parseInt() to throw (Namely NumberFormatException). That way if something else goes wrong, the exception won't be caught, and you will get a clear error drawing your attention to the problem (rather than merely returning false and moving on).

Note that I also got rid of the value variable, since we don't actually use it for anything.

[Junilu]: 2. You should not have a return statement in a catch block.

Generally I'd agree. But this is a special case where we understand exactly why that specific exception was thrown, it's expected, and we know that returning false really is the result we want. Returning from the catch makes perfect sense here, I think. Some peoople may feel compelled to rewrite the code above to eliminate the multiple returns, but I don't much care about that.
[ December 14, 2005: Message edited by: Jim Yingst ]
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[Junilu]: Note that it doesn't use exceptions either

And note how long the method is. That's fine if you use an existing library that's well-tested, but if you're writing your own method I think it's much simpler and more reliable to rely on code from the standard classes. And note that in most cases, you want to be able to store the number in a variable of some sort - int, long, BigInteger, whatever. If you use parseInt() or hasNextInt() it's quite easy to modify the code to work with long or BigInteger instead. If you're using java.util.regex or otherwise parsing the string by hand, it may not be as trivial to code in size limitations.
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Wouldn't it be preferable to handle the exception explicity as bad input rather than hide it in isValidNum()?




While the second snippet may look cleaner, it calls Integer.parseInt() twice.
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well typically I'd want the one that looks cleaner, and not care much about calling parse twice. Unless there's a reason to optimize performance in this particular area - in which case it would probably depend on how common bad input is. If performance here is critical and bad input is common (or at least, not too unusual), then it's worthwhile to avoid creating and throwing an exception for bad input. If bad input is rare, then optimize for the happy path and call parseInt() only once.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!