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

URL decode a string but log an error after second exception

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

Problem

I have a method which does URL decoding on the string value passed along with using Charset:

public String decodeValue(String value, Charset charset) {
    if (!Strings.isNullOrEmpty(value)) {
      try {
        value = URLDecoder.decode(value, charset.name());
      } catch (UnsupportedEncodingException ex) {
        // log error
        return null;
      }
    }
    return value;
  }


Now if URLDecoder.decode throws UnsupportedEncodingException the first time, then I want to run the same value against these three lines:

value = value.replaceAll("%(?![0-9a-fA-F]{2})", "%25");
value = value.replaceAll("\\+", "%2B");
value = URLDecoder.decode(value, charset.name());


And if then again URLDecoder.decode line throws exception second time, then I will log the error but only second time and return null value otherwise return the value which is decoded so I came up with this:

public String decodeValue(String value, Charset charset) {
    if (!Strings.isNullOrEmpty(value)) {
      try {
        value = URLDecoder.decode(value, charset.name());
      } catch (UnsupportedEncodingException ex) {
        try {
          value = value.replaceAll("%(?![0-9a-fA-F]{2})", "%25");
          value = value.replaceAll("\\+", "%2B");
          value = URLDecoder.decode(value, charset.name());
        } catch (UnsupportedEncodingException uex) {
          // log error
          return null;
        }
      }
    }
    return value;
  }


I'd like to see if there is any better way to write the above code. As of now, it duplicates the code.

Solution

First, let's move the replacements into a separate fixErrors method for readability.

String fixErrors(String value) {
    return value
        .replaceAll("%(?![0-9a-fA-F]{2})", "%25")
        .replaceAll("\\+", "%2B");
}


Now, my solution would be to write a method that returns the result of URLDecoder.decode as an Optional. Then, instead of nesting try/catch statements, we can just use Optional::or. This should look something like

public String decodeValue(String value, Charset charset) {
    if (!Strings.isNullOrEmpty(value)) {
        return tryDecode(value, charset)
            .or(() -> tryDecode(fixErrors(value), charset))
            .orElse(null);
    }
    return value;
}

Optional tryDecode(String value, Charset charset) {
    try {
        return Optional.of(URLDecoder.decode(value, charset.name()));
    } catch (UnsupportedEncodingException ex) {
        return Optional.empty();
    }
}


Unfortunately, this solution precludes logging details of only the second error. To do this effectively, we could add error handling to tryDecode by passing it a Consumer. Then we would have:

public String decodeValue(String value, Charset charset) {
    if (!Strings.isNullOrEmpty(value)) {
        return tryDecode(value, charset, ex -> {})
        .or(() -> tryDecode(fixErrors(value), charset, ex -> {
                // log ex
            }))
        .orElse(null);
    }
    return value;
}

Optional tryDecode(String value, Charset charset,
                   Consumer exConsumer) {
    try {
        return Optional.of(URLDecoder.decode(value, charset.name()));
    } catch (UnsupportedEncodingException ex) {
        exConsumer.accept(ex);
        return Optional.empty();
    }
}


Hopefully this solution meets your requirements.

Code Snippets

String fixErrors(String value) {
    return value
        .replaceAll("%(?![0-9a-fA-F]{2})", "%25")
        .replaceAll("\\+", "%2B");
}
public String decodeValue(String value, Charset charset) {
    if (!Strings.isNullOrEmpty(value)) {
        return tryDecode(value, charset)
            .or(() -> tryDecode(fixErrors(value), charset))
            .orElse(null);
    }
    return value;
}

Optional<String> tryDecode(String value, Charset charset) {
    try {
        return Optional.of(URLDecoder.decode(value, charset.name()));
    } catch (UnsupportedEncodingException ex) {
        return Optional.empty();
    }
}
public String decodeValue(String value, Charset charset) {
    if (!Strings.isNullOrEmpty(value)) {
        return tryDecode(value, charset, ex -> {})
        .or(() -> tryDecode(fixErrors(value), charset, ex -> {
                // log ex
            }))
        .orElse(null);
    }
    return value;
}

Optional<String> tryDecode(String value, Charset charset,
                   Consumer<UnsupportedEncodingException> exConsumer) {
    try {
        return Optional.of(URLDecoder.decode(value, charset.name()));
    } catch (UnsupportedEncodingException ex) {
        exConsumer.accept(ex);
        return Optional.empty();
    }
}

Context

StackExchange Code Review Q#151441, answer score: 2

Revisions (0)

No revisions yet.