Method always returns true?

Question

I have a method that is being called to validate that an IP address is assignable. No matter what I pass to it, it is always returning true. What do I need to set the return as to get this method working properly?

public boolean checkValidIPClass(String x) {
    for (int i = 0; i < 4; i++) {
        try {
            if (retString.equals("A")) {
                if ((intParts[1] == 0) && (intParts[2] == 0) && (intParts[3] == 0))
                    return false;
                if ((intParts[1] == 255) && (intParts[2] == 255) && (intParts[3] == 255))
                    return false;
            }
            if (retString.equals("B")) {
                if ((intParts[2] == 0) && (intParts[3] == 0))
                    return false;
                if ((intParts[2] == 255) && (intParts[3] == 255))
                    return false;
            }
            if (retString.equals("C")) {
                if (intParts[3] == 0)
                    return false;
                if (intParts[3] == 255)
                    return false;
            }
        } //ends try
        catch (NumberFormatException nfe) {
            return false;
        } //ends catch
    } //ends for
    return true;
}

retString is making it to the method and is a string that was returned from another method that checks what class the IP address is and assigns it, this was verified with a print statement. Thanks!

EDIT: How has this been answered and downvoted? My question wasn't about comparing the strings, it was about the method always returning true even when I know the if statements should be catching the error and returning false?

EDIT2: Updated my code.

This question is marked as duplicate, but to be absolutely clear - the way you are doing your string comparisons in the outer if statements will always return false. You need to use `string.equals(otherString)` instead. — CodeBlind, Apr 10 '15 at 17:13
We don't know that's the problem for sure, If the OP passes a literal string the `==` operator should work (but it should still be `.equals()`). The problem could be with `intParts`. Also I don't know what that `catch NumberFormatException` is doing there. — Paul Boddington, Apr 10 '15 at 17:18
Okay, I cleaned that up and used equals. intParts array is making it to the method, I can print the indexes and see the correct values, however the method is still always returning true. Why is this? Thanks for the help! — still2blue, Apr 10 '15 at 17:32
@Pshemo Can you reopen this? Apparently `==` wasn't the problem. — Paul Boddington, Apr 10 '15 at 17:36
@still2blue Can you post the new version with `equals`? Can you also show us what `System.out.println(Arrays.toString(intParts));` gives? — Paul Boddington, Apr 10 '15 at 17:40
@still2blue I will reopen your question if you edit it to show code which is not using `==` to compare strings but is using `equals` method. Currently none of your `if(str1=="foo")` conditions can be true (unless `str1` will be also literal or manually interned string which was created at runtime) so you will not enter any block which it describes. So again, if you will be able to reproduce this problem with condition like `if(str.equals("foo"))` question will be opened. — Pshemo, Apr 10 '15 at 17:41
Looking at the code, it makes *absolutely no sense*. Theres an argument "x" to the method that is never used, a for loop without anything that remotely requires a loop in its body, a catch for an exception that can't be thrown and the method uses state "retString" and "intParts" which isn't defined nor initialized in the code shown. And you want *us* to tell you what the problem is? — Durandal, Apr 10 '15 at 17:44
@pbabcdefp If `==` wasn't the problem then question shouldn't be closed *as duplicate*, but *as off-topic* since "*Questions seeking debugging help ("why isn't this code working?") must include the desired behavior, a specific problem or error and the shortest code necessary to **reproduce** it in the question itself.*" currently we have no idea about how OP is using this method and what data are used, so we can't even try reproducing *the* problem. — Pshemo, Apr 10 '15 at 17:49
@pbabcdefp "Ok" as in it might compile? Yes. "Ok" as in "is an answerable question"? No. — Durandal, Apr 10 '15 at 17:49
I reopened your question, but it is still not answerable until you post [SSCCE](http://sscce.org/) — Pshemo, Apr 10 '15 at 17:51
I'm sorry guys I am new to Java and obviously no expert. The retString is being passed into the method and the array has already been defined and initialized in the method that called it. I am passing the IP address (inside the array parts) as `121.0.0.0` and `retString` is `"A"` in this case. I am still trying to figure out why it isn't meeting the condition in the if statement where `retString.equals("A")` and returning false to the method I am calling it from. Sorry for my ignorance and thanks for the help! — still2blue, Apr 10 '15 at 17:52
The method calling it is making sure the address is in valid format. It then calls this method to make sure it's assignable. If this method returns false, then I will set the other method to also return false, therefore displaying the address is invalid. — still2blue, Apr 10 '15 at 17:54
@still2blue The comments of Pshemo and Durandal are all valid. It is very difficult to understand what you are trying to do. Perhaps you could explain in the question? Can you post a complete example (showing where you call the method and where you declare and initialise all those variables) that we can run? — Paul Boddington, Apr 10 '15 at 17:57
Can you build a runnable example? This should also contain what `retString` is (type and initial value) and what `intParts` is (type and value). And can you tell my, why you use a loop there? — Tom, Apr 10 '15 at 17:59

score 2 · Answer 1 · answered Apr 10 '15 at 18:49

I don't get why you're doing a loop, but you could try this:

public boolean checkValidIPClass(String ipClass, String ipAddress)
{
    if (ipClass.contentEquals("A"))
    {
        if (ipAddress.endsWith("0.0.0") || ipAddress.endsWith("255.255.255"))
            return false;
    }
    else if (ipClass.contentEquals("B"))
    {
        if (ipAddress.endsWith("0.0") || ipAddress.endsWith("255.255"))
            return false;
    }
    else if (ipClass.contentEquals("C"))
    {
        if (ipAddress.endsWith("0") || ipAddress.endsWith("255"))
            return false;
    }

    return true;
}

Since you're just checking the ending array parts of the IP address, you don't need to break it into an array, just leave it as a string.

And keep in mind that this would only satisfy IPv4 formatted IP addresses. It will not work for IPv6 formatted addresses

Method always returns true?

1 Answers1