Adding tournament with params

[BrentonPoke]

Ok, checked and re-checked with multiple games. It works in my test program.

[BrentonPoke]

I just noticed I mis-named the branch. ¯_(ツ)_/¯

[MatthijsKok]

Why not use an URIBuilder? https://stackoverflow.com/a/26641952

Also, using javafx's Pair is not very clean. Is something like a HashMap possible?

[skamoen]

I implemented something very similar for Matches already. That way you don't have to "guess" the parameters, but they'll be specified in the client already.

See here for the matches implementation: https://github.com/AreaFiftyLAN/toornament-client/blob/feature-matches/src/main/java/ch/wisv/toornament/concepts/Matches.java

The URIBuilder Matthijs mentioned is even cleaner, I'll change it for the Matches but you can combine it here :)

[BrentonPoke]

I can't find whichever package URIBuilder requires in Maven Central, so I can't use it. And javaFX's pair works just like C++'s std::pair, which is very simple.

Plus, I don't know if URIBuilder will know to use "?" to begin the parameters, then "&" for subsequent ones. This is why I didn't use okhttp3's HttpUrl.

[BrentonPoke]

Hold on; I tried something just now.

HttpUrl.Builder b = new HttpUrl.Builder();
       b.scheme("https")
.host("www.google.com")
.addQueryParameter("discipline","overwatch")
.addQueryParameter("country", "US");

This code resulted in this: "https://www.google.com/?discipline=overwatch&country=US"
It's similar to URIBuilder for this purpose, so does everybody want to do this?

It should work in a simple for-loop.

[skamoen]

I just did this for the Matches branch as well, seems to work well and looks good.

Instead of a for loop, I'd prefer something like this: https://github.com/AreaFiftyLAN/toornament-client/blob/feature-matches/src/main/java/ch/wisv/toornament/concepts/Matches.java#L27

So you don't have to manually enter the parameter names.

[BrentonPoke]

You would have to have a much larger call defined and still must know the allowable string values for some of the parameters. Depending upon how much the library is supposed to do out of the box, you'd have to hard-code into the library a lot of possible values as options for something like the tournaments endpoint. You could do that, but if so, that would likely mean a parameter options builder object. Sent from my Verizon, Samsung Galaxy smartphone null

[skamoen]

I covered a lot of it already in the Matches branch I think. If you add your for-loop implementation in this PR, I'll add a predefined method after I finish the Matches branch, ok?

[BrentonPoke]

Just so we're clear, does this mean throwing out the Pair<String,String> as an input? Because the for-loop basically does this:

HttpUrl.Builder b = new HttpUrl.Builder();
       b.scheme("https");

for (Pair<String, String> params : paramsList) {
      b.host("www.google.com")
.addQueryParameter("params.L",params.R)
        }

What would the new call look like?

[skamoen]

I'd prefer not to have a JavaFX package as a dependency. It might just be a single class, but it's not meant for this type of thing. How about a simple HashMap<String, String>? You can also iterate over that as well with

for (Map.Entry<String, String> entry : map.entrySet()) {
   b.addQueryParameter(entry.getKey() , entry.getValue())
}

[BrentonPoke]

That's fine; I just want to know what the function prototype would look like. Because you're still going to have a build a HashMap, and to do that, will need to know what the parameters for the endpoints are.

[skamoen]

You mean this?

public List<Tournament> getTournamentsWithParams(Map<String, String> parameterMap) {
for (Map.Entry<String, String> entry : parameterMap.entrySet()) {
   b.addQueryParameter(entry.getKey() , entry.getValue())
}

Sorry if I'm misunderstanding your question. It's been a long day of coding for me 😴

[BrentonPoke]

Oh, well that's basically what I have now. I can make that change. I thought you wanted to do something that defined every parameter option in the API with a method.

[skamoen]

That's what I was trying to say in #13 (comment). In the Matches branch, I already created a number of enums and date formatters to deal with the parameters. Once I'm done with that branch, I'll re-use those classes to create a similar method for Tournaments.

The signature would be something like this:
getTournament(String discipline, TournamentStatus status, Boolean featured, Boolean online, String countryCode, Date startAfter, Date startBefore, Date endAfter, Date endBefore, Sort sort, String name)

They'd be all nullable so you can create a query using only the parameters you want.

[skamoen]

Shouldn't this be .addPathSegment("v1/tournaments") as we're not really encoding strange characters?

[BrentonPoke]

I think you can do that, but I interpreted a path segment as a URL segment

[skamoen]

Doesn't really matter anyway. I'm currently working on the Matches parameter thing, and I think you're right that using a parameter builder object is better than a method containing all parameters. PR should be done today :)