Parsing prospero parameters

Question

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 <code>null</code> sein.
 * @param name Der Name des Parameters, darf kein '=' enthalten, darf nicht <code>null</code>
 * sein.
 * @return Den Wert des Parameters oder
 * <dl>
 * <dt><code>null</code></dt>
 * <dd>wenn der Parameter ohne <code>=</code> angegeben wurde.</dd>
 * <dt><code>error</code>-Parameter</dt>
 * <dd>wenn kein solcher Parameter gefunden wurde</dd>
 * </dl>
 */
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:[email protected]/a.a?a=a#a"), "a", null) == null;
assert URLTools.getProsperoParam(new URL("http://a:[email protected]/a.a;a=4?a=a#a"), "a", null).equals("4");
assert URLTools.getProsperoParam(new URL("http://a:[email protected]/a.a;a=4=a?a=a#a"), "a", null).equals("4=a");
assert URLTools.getProsperoParam(new URL("http://a:[email protected]/a.a;a=4=a&m=3?a=a#a"), "m", null).equals("3");
String missingString = "Missing!";
assert URLTools.getProsperoParam(new URL("http://a:[email protected]/a.a;a=4=a&m=3?a=a#a"), "e", missingString) == missingString;
assert URLTools.getProsperoParam(new URL("http://a:[email protected]/a.a;e?a=a#a"), "e", "asdf") == null;
assert URLTools.getProsperoParam(new URL("http://a:[email protected]/a.a;e=?a=a#a"), "e", null).equals("");

• tim: you might want to mention what a Prospero is (and what Prospero-Parameter and Prospero-Part specifically are). I tried google, but the third result is already this question. It seems to be just a normal URL parameter?
• ferada: Do you actually need to distinguish the edge cases (parameter without value, empty value, value with = included) and can't just throw them out as invalid?
• Grim: @ferada yes i need them. A empty value != missing value != missing key != not set.

Answer 1

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:[email protected]/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.

• Grim: I realy need == on Strings(!), its not allowed to replace it with equals because i would not be able to check if the value is a real parameter-value or my error-value.
• h.j.k.: @PeterRader, for IAE vs NPE, one school of thought is to consult what the Javadocs have to say for both exceptions. IAE is "thrown to indicate that a method has been passed an illegal or inappropriate argument", while NPE is "thrown when an application attempts to use null in a case where an object is required". It certainly seems that if null is an inappropriate method argument for your method, throwing IAE would be clearer. For more info: stackoverflow.com/questions/3881/…
• Grim: This is not an so important discussion, i can do it or i can let it be. If you guys gets upset, i'll accept it. Hey, if i like to become a ranter i should use Both: IAE(NPE). Even the implementation of .split( dont care about the more specific-qualified IAE(NPE) but i have to, am i something better?! Well i feel dirty if i throw a more-specific-qualified-exception without write it into the JavaDoc. But if i write it into the JavaDoc, it must match the Method's signature. And then Checkstyle will critizise me to have unchecked exceptions in the Method's signature. Life isnt Ponyhof. ;)
• tim: @PeterRader you are right, even oracle handles this quite inconsistently. Matcher:usePattern does check, and throws an IAE, Scanner:hasNext does check, but throws a NPE. I didn't find any cases for public methods that don't check at all, but they may exist (I would prefer checking, some methods might produce side-effects otherwise). It seems that NPE is actually more common; there even exists a method for it: Objects:requireNonNull: This method is designed [for] parameter validation in methods
• h.j.k.: @PeterRader sorry if my comment sounded like IAE must be used over NPE, there's really no right or wrong answer for this holy war within Java. :) Take whichever makes your method signature more consistent, and it seems like you have already justified your usage. All is good...