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

Efficient URL Escape (Percent-Encoding)

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

Problem

Here I have a simple algorithm to percent-encode any string.

(Specification from Wikipedia; note that this is not compatible with URLEncoder.encode())

Is this an efficient solution to the problem?

Using a StringBuilder should be efficient, but it doesn't seem great since every character is added to the StringBuilder individually. Could this have any significant impact?

This method works only for ASCII characters; that is sufficient for my use case.

```
private static String urlEscape(String toEscape){
//if null, keep null (no gain or loss of safety)
if (toEscape==null)
return null;

StringBuilder sb=new StringBuilder();
for (char character: toEscape.toCharArray())//for every character in the string
switch (character){//if the character needs to be escaped, add its escaped value to the StringBuilder
case '!': sb.append("%21"); continue;
case '#': sb.append("%23"); continue;
case '$': sb.append("%24"); continue;
case '&': sb.append("%26"); continue;
case '\'': sb.append("%27"); continue;
case '(': sb.append("%28"); continue;
case ')': sb.append("%29"); continue;
case '*': sb.append("%2A"); continue;
case '+': sb.append("%2B"); continue;
case ',': sb.append("%2C"); continue;
case '/': sb.append("%2F"); continue;
case ':': sb.append("%3A"); continue;
case ';': sb.append("%3B"); continue;
case '=': sb.append("%3D"); continue;
case '?': sb.append("%3F"); continue;
case '@': sb.append("%40"); continue;
case '[': sb.append("%5B"); continue;
case ']': sb.append("%5D"); continue;
case ' ': sb.append("%20"); continue;
case '"': sb.append("%22"); continue;
case '%': sb.append("%25"); continue;
case '-': sb.append("%2D"); continue;
case '.': sb.append("%2E"); continue;
case '': sb.append("%3E"); continue;
case '\\': sb.append("%5C"); con

Solution

You are in dire need of a lookup table. What you want to do is define a mapping somewhere else, and then just percent-encode all characters you have mapped. This gets rid of that unwieldy, huge and btw. hacky switch-statement.

consider the following:

StringBuilder sb = new StringBuilder(toEncode.length());
for (Character c : toEncode.toCharArray()) {
    if (MAPPING.containsKey(c)) {
       sb.append(MAPPING.get(c));
    } else {
       sb.append(c);
    }
}


This should be equally fast, if not faster. Also it makes use of a MAPPING you can change, without significantly affecting how this method works. btw. I am prespecifying the StringBuilder's capacity, since it's more efficient when the backing collection in there does not need to be resized too often.

Oh and another thing. Your switch-case only works by sheer luck. continue; is not the correct keyword to use there, instead you should rely on break; that allows you to add more work in the loop body, which is currently more or less impossible.

Code Snippets

StringBuilder sb = new StringBuilder(toEncode.length());
for (Character c : toEncode.toCharArray()) {
    if (MAPPING.containsKey(c)) {
       sb.append(MAPPING.get(c));
    } else {
       sb.append(c);
    }
}

Context

StackExchange Code Review Q#102591, answer score: 8

Revisions (0)

No revisions yet.