Parsing prospero parameters

Prospero is a URI scheme. There you have fields and values.

Could you check this code and give common suggestions or even test cases for JUnit tests?

(The documentation is in German.)

/**
 * Gibt einen Prospero-Parameter aus der URL zurück. Dabei wird nur der Prospero-Part geprüft.
 * 
 * @param url Die URL, darf nicht null sein.
 * @param name Der Name des Parameters, darf kein '=' enthalten, darf nicht null
 *            sein.
 * @return Den Wert des Parameters oder
 *         
 *         null
 *         wenn der Parameter ohne = angegeben wurde.
 *         error-Parameter
 *         wenn kein solcher Parameter gefunden wurde
 *         
 */
public static String getProsperoParam(URL url, String name, String error) {
    String path = url.getPath();
    String[] split = path.split(";", 2);
    if (split.length == 2) {
        String params = split[1];
        for (String param : params.split("&")) {
            String[] paramParts = param.split("=", -1);
            if (paramParts.length == 1) {
                if (name.equals(paramParts[0])) {
                    return null;
                }
            }
            if (paramParts.length == 2) {
                if (name.equals(paramParts[0]))
                    return paramParts[1];
            }
            if (paramParts.length > 2) {
                if (name.equals(paramParts[0])) {
                    return param.substring(name.length() + 1);
                }
            }
        }
    }
    return error;
}

I successfully tested these cases:

```
assert URLTools.getProsperoParam(new URL("http://a:a@a.a.a/a.a?a=a#a"), "a", null) == null;
assert URLTools.getProsperoParam(new URL("http://a:a@a.a.a/a.a;a=4?a=a#a"), "a", null).equals("4");
assert URLTools.getProsperoParam(new URL("http://a:a@a.a.a/a.a;a=4=a?a=a#a"), "a", null).equals("4=a");
assert URLTools.getProsperoParam(new URL("http://a:a@a.a.a/a.a;a=4=a&m=3?a=a#a"), "m", null).equals("3");
String missingString = "Missing!";
assert URLTools.

Check input

Your comments mention that url shouldn't be null, so it's best to make sure that it really isn't:

if (url == null) {
        throw new IllegalArgumentException("url cannot be null");
    }

Nested if statements

if (paramParts.length == 1) {
            if (name.equals(paramParts[0])) {
                return null;
            }
        }

You have this principle three times. It can be written as:

if (paramParts.length == 1 && name.equals(paramParts[0])) {
            return null;
        }

Although I would rewritten your if statements like this:

if (paramParts.length > 0 &&  name.equals(paramParts[0])) {
            switch(paramParts.length) {
                case 1:
                    return null;
                case 2:
                    return paramParts[1];
                default:
                    return param.substring(name.length() + 1);
            }
        }

And are you sure that paramParts.length == 2 (now case 2) is really needed? I think the other cases cover it.

Use early return

If you return early, you can reduce nesting:

if (split.length != 2) {
    return error;
}

Tests

The specification for prosperos says:


each field/value pair is separated from each other and from the rest of the URL by a ";" (semicolon).

You are never testing if this would work, and it wouldn't:

String prosperoParam = prospero.getProsperoParam(new URL("http://a:a@a.a.a/a.a;a=a;b=b"), "b", null);
assertEquals(prosperoParam, "b");

If you want to change the spec, I would comment on it in the Java doc (something like: See RFC 4157, but note that this implementation uses '&' instead of ';' for the separation of field/value pairs (it still does use ';' for the separation between the first field/value pair and the rest of the URL).

Also, sometimes you use == to compare strings in your tests, I would replace it with equals.

StackExchange Code Review Q#66738, answer score: 6