Java Query String Parser

Question

I have written a Query String Parser to Parse a given query string. Is there any enhancement I can do?

/**
 * Class QueryStringParser is used to parse Query String and to get value(s) of
 * the given parameter key.
 * 
 * @author TapasB
 */
public class QueryStringParser {
    private static class NameValuePair {
        final String key;
        final String value;

        NameValuePair(String key, String value) {
            this.key = key;
            this.value = value;
        }
    }

    public static final String DEFAUL_CHARSET = "UTF-8";

    private Charset charset;
    private List<NameValuePair> query = new ArrayList<NameValuePair>();

    /**
     * Constructor QueryStringParser initialises a new instance of this class
     * with the given <tt>queryString</tt>. By default it uses the "UTF-8"
     * Character Encoding.
     * 
     * @author TapasB
     * @param queryString
     *            The Query String to parse
     * @throws NullPointerException
     *             If the given <tt>queryString</tt> is null
     */
    public QueryStringParser(String queryString) throws NullPointerException {
        this(queryString, DEFAUL_CHARSET);
    }

    /**
     * Constructor QueryStringParser
     * 
     * @author TapasB
     * @param queryString
     *            The Query String to parse
     * @param charsetName
     *            The {@link Charset} needs to be used to parse
     * @throws NullPointerException
     * @throws IllegalCharsetNameException
     *             If the given <tt>charsetName</tt> name is illegal
     * @throws IllegalArgumentException
     *             If the given <tt>charsetName</tt> is null
     * @throws UnsupportedCharsetException
     *             If no support for the named <tt>charsetName</tt> is available
     *             in this instance of the Java virtual machine
     */
    public QueryStringParser(String queryString, String charsetName) throws NullPointerException, IllegalCharsetNameException, IllegalArgumentException, UnsupportedCharsetException {
        if (queryString == null) {
            throw new NullPointerException("Query String is Null.");
        }

        this.charset = Charset.forName(charsetName);
        parse(queryString);
    }

    // This method is used to parse the query string
    private void parse(String queryString) {
        for (String pair : queryString.split("&")) {
            int idxOfEqual = pair.indexOf("=");

            if (idxOfEqual < 0) {
                addElement(pair, "");
            } else {
                String key = pair.substring(0, idxOfEqual);
                String value = pair.substring(idxOfEqual + 1);
                addElement(key, value);
            }
        }
    }

    // This method adds the given key and value into the List query.
    // Before adding it decodes the key and value with the given charset.
    public void addElement(String key, String value) {
        if (key == null || value == null) {
            throw new NullPointerException("Key or Value is Null");
        }

        try {
            String charsetName = charset.name();
            query.add(new NameValuePair(URLDecoder.decode(key, charsetName), URLDecoder.decode(value, charsetName)));
        } catch (Exception ignore) {
        }
    }

    /**
     * Method getParameterValue returns the value associated with the given key
     * 
     * @author TapasB
     * @param key
     *            The key for which the value is needed
     * @return The associated value to the given key. If the given key is not
     *         found then it returns null
     */
    public String getParameterValue(String key) {
        for (NameValuePair pair : query) {
            if (pair.key.equals(key)) {
                return pair.value;
            }
        }

        return null;
    }

    /**
     * Method getParameterValues returns the List of values associated with the
     * given key
     * 
     * @author TapasB
     * @param key
     *            The key for which the List of values are needed
     * @return The associated List of values to the given key. If the given key
     *         is not found then it returns an empty list
     */
    public List<String> getParameterValues(String key) {
        List<String> list = new ArrayList<String>();

        for (NameValuePair pair : query) {
            if (pair.key.equals(key)) {
                list.add(pair.value);
            }
        }

        return list;
    }
}

Test:

public static void main(String[] args) {
    QueryStringParser queryStringParser = new QueryStringParser("IdcService=SS_GET_PAGE&siteId=DAM_MH&ssUrlType=2&siteRelativeUrl=index.htm&isDam=1&isDam=2");
    System.out.println(queryStringParser.getParameterValue("IdcService"));
    System.out.println(queryStringParser.getParameterValues("isDam"));
}

Answer 1

Your DEFAUL_CHARSET is a String, why not Charset? You can use StandardCharsets.UTF_8 (if you use Java 1.7) and avoid the use of Charset.forName too. With Java < 1.7 you could use Guava, but you can keep the current way if you can't upgrade.

throws NullPointerException, IllegalCharsetNameException, IllegalArgumentException, UnsupportedCharsetException

The exceptions IllegalCharsetNameException and UnsupportedCharsetException both extends IllegalArgumentException so you don't need to declare them in the throws list.

public QueryStringParser(String queryString, Charset charsetName) throws NullPointerException, IllegalArgumentException {

And why NullPointerException and IllegalArgumentException? You don't need to provide this at all.

You want to be clear as much possible about which exceptions a method could throw? Remember then you should keep updated documentation and code too.

The method addElement is public but don't have documentation just some comments

// This method adds the given key and value into the List query.
// Before adding it decodes the key and value with the given charset.

You seems to care a lot about code documentation (great) but don't forget to document all public methods

In your addElement you catch

} catch (Exception ignore) {
}

Why you ignore everything? It will ignore NPE, SomeRandomException etc. And it's very very very very very bad to see. Because it will be very ugly to debug! Because you will never know what caused your bug until you don't follow it line for line.

Catch directly UnsupportedEncodingException and ignore it if you already know that can't happen or better:

try {
    query.add(new NameValuePair(URLDecoder.decode(key, charsetName), URLDecoder.decode(value, charsetName)));
} catch (UnsupportedEncodingException e) {
    throw new AssertionError(e);
}

I think the best thing to use in your case is a Map and since you want to have multiple values with the same key, a Multimap of Guava could help you.

Your parse could look better and avoid too much nesting:

private void parse(String queryString) {
    for (String pair : queryString.split("&")) {
        int idxOfEqual = pair.indexOf("=");

        if (idxOfEqual < 0) {
            addElement(pair, "");
            continue;
        }

        String key = pair.substring(0, idxOfEqual);
        String value = pair.substring(idxOfEqual + 1);

        addElement(key, value);
    }
}

The method indexOf returns -1 if not founds, so why not:

if (idxOfEqual == -1) {
   // blabla
}

Your line query.add(new NameValuePair(URLDecoder.decode(key, charsetName), URLDecoder.decode(value, charsetName))); in addElement is long to see in one line.

I always like to keep the code not so long to see, something like maybe?

query.add(new NameValuePair(
        URLDecoder.decode(key, charsetName), 
        URLDecoder.decode(value, charsetName)
));

But it could be a personal thing.

If you use Java 1.7 you can use diamond operator.

I would move the most important constructor at the top (QueryStringParser(String queryString, Charset charsetName)) and then the help constructors:

public QueryStringParser(String queryString, Charset charsetName) {
    if (queryString == null) {
        throw new NullPointerException("Query String is Null.");
    }

    this.charset = charsetName;
    parse(queryString);
}

public QueryStringParser(String queryString) {
    this(queryString, DEFAUL_CHARSET);
}

I would say NameValuePair could be generic, but in your code i don't think it needs to be.

You document UnsupportedCharsetException, IllegalArgumentException and IllegalCharsetNameException but why? The point of use JDK Exception is the fact that the programmer already knows when they appears to it's obvious that if i see IllegalArgumentException the problem is one of the arguments i provided and the message will help me to know what was.

Often the documentation is not available, so put the important informations in the right places.

---

Random notes

Not so important just some things:

• Some free JUnit test cases for you

  public class QueryStringParserTest {
  
      @Test
      public void t1() {
          QueryStringParser queryStringParser = new QueryStringParser("IdcService=SS_GET_PAGE&siteId=DAM_MH&ssUrlType=2&siteRelativeUrl=index.htm&isDam=1&isDam=2");
          assertEquals("SS_GET_PAGE", queryStringParser.getParameterValue("IdcService"));
      }
  
      @Test
      public void t2() {
          QueryStringParser queryStringParser = new QueryStringParser("IdcService=SS_GET_PAGE&siteId=DAM_MH&ssUrlType=2&siteRelativeUrl=index.htm&isDam=1&isDam=2");
          assertEquals("DAM_MH", queryStringParser.getParameterValue("siteId"));
      }
  
      @Test
      public void t3() {
          QueryStringParser queryStringParser = new QueryStringParser("IdcService=SS_GET_PAGE&siteId=DAM_MH&ssUrlType=2&siteRelativeUrl=index.htm&isDam=1&isDam=2");
  
          List<String> result = new ArrayList<>();
          result.add("1");
          result.add("2");
  
          assertEquals(result, queryStringParser.getParameterValues("isDam"));
      }
  }
  

  the names are random!

• Recently, i become a maniac of @Nullable and @NotNull annotations which is very helpful because you will let know to others if you method could return a null of they can be sure that it will never be null. And it's useful to you too because you could not remember if a method could return null and let you think "Why this method returns null? Is there are any reason?"