Add HttpUrl class #49
Conversation
|
fyi @ellisjoe |
|
nvm, will have to tweak this to be correct. |
| import java.util.List; | ||
|
|
||
| /** A simplistic URL builder, not tuned for performance. */ | ||
| public final class HttpUrl { |
There was a problem hiding this comment.
also might consider just making the top-level class the builder and have the 'build' method be toUrl
| private String host; | ||
| private int port = -1; | ||
| private List<String> pathSegments = new ArrayList<>(); | ||
| private List<String> queryNamesAndValues = new ArrayList<>(); // alternating (name, value) pairs |
There was a problem hiding this comment.
might make more sense for this to be a Multimap
There was a problem hiding this comment.
I suspect the list construction has fewer allocations; I think it's OK since it's just used internally?
|
|
||
| private static String encodePath(List<String> pairs) { | ||
| StringBuilder result = new StringBuilder(); | ||
| for (int i = 0; i < pairs.size(); i += 1) { |
There was a problem hiding this comment.
possibly nicer to write this as:
Joiner.on("/").join(paths)There was a problem hiding this comment.
done here, but not below to save object allocations
| return URLEncoder.encode(string, StandardCharsets.UTF_8); | ||
| } | ||
|
|
||
| private static String encodePath(List<String> pairs) { |
|
you may want to consider looking at URI, the docs on URL say this:
|
|
problem with the URI class is that it cannot distinguish between |
|
take another look please, @markelliot |
dialogue-java-client/src/main/java/com/palantir/dialogue/UrlBuilder.java
Show resolved
Hide resolved
dialogue-java-client/src/main/java/com/palantir/dialogue/UrlBuilder.java
Show resolved
Hide resolved
|
take another look please, @markelliot |
No description provided.