HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Check if a String is a valid Double

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checkdoublestringvalid

Problem

For a class, I need to ask the user for a double. To verify that the number can be safely passed to Double.parseDouble, I wrote the following function. I'd like to have it heavily scrutinized; including the comments.

-
Are my comments helpful? They're more directed towards the marker than a maintainer down the road. Should I phrase them this way if it's for a class, and not "real code" that will need to be maintained later?

-
Is my assertion about the nature of charAt correct? Should I be caching str.charAt(i) in the loop?

-
I was rather proud of the first validity check, but found it to be kind of ugly when I looked at it the next day. Is there any better way that it could be written?

-
Anything else that could be improved. Be critical please!

As @h.j.k pointed out, this would be entirely easier by just catching the exception that Double.parseDouble throws, but there's no fun in that! I wanted the exercise.

```
import static java.lang.Character.*;

public static boolean strIsNumeric(String str) {
//If the string is empty, or if it isn't a '-' or digit, return false.
//We need a check like this or a prefixed '-' will prevent it from
// being recognized as a negative number.
//This is safe because logical-OR "short-circuits", the right hand will only
// evaluate if the left is false. If this wasn't the case, the calls to "charAt(0)"
// would cause an exception on empty String.
if ( str.isEmpty() || (str.charAt(0) != '-' && !isDigit(str.charAt(0))) ) {
return false;
}

//We're skipping the first char since it was checked above.
boolean foundADecimal = false;
for (int i = 1; i < str.length(); i++) {
//To avoid repeated evaluations
char c = str.charAt(i);

//Check to see if there's at most 1 decimal
if ( !foundADecimal && c == '.' ) {
foundADecimal = true;

//We've "ver

Solution

There are three validation failures (ignoring case sensitivity) that Double.parseDouble() would have accepted otherwise:

  • A starting positive sign (+1.23).



  • A starting decimal point (.123).



  • The E notation aka scientific notation (1.23E4).



As for your comments... they should explain the why, not the how. The comments you have now are only explaining the how, so you should consider removing them when you get the chance to write code outside of academic exercises in the future...

Variable names... it depends. i is quite well-understood, but some may frown upon a single c to represent a character, or even str for your method argument... I'm not suggesting to be overly descriptive (currentCharacterOfMethodArgument is an example), but maybe an actual English word (current?) might be slightly more readable for your marker.

And just for completeness, remember to do a null check at the start too, before str.isEmpty(). :)

edit And since you asked about it, the implementation of charAt(int) is just:

public char charAt(int index) {
    if ((index = value.length)) {
        throw new StringIndexOutOfBoundsException(index);
    }
    return value[index];
}


The only 'evaluation' here is the if statement for out-of-bounds check, which I don't think is that significant/performance-hurting (if you're alluding to this indirectly as well). It's fine both ways whether you want to 'cache' it first.

Code Snippets

public char charAt(int index) {
    if ((index < 0) || (index >= value.length)) {
        throw new StringIndexOutOfBoundsException(index);
    }
    return value[index];
}

Context

StackExchange Code Review Q#91347, answer score: 6

Revisions (0)

No revisions yet.