take empty-array into consideration

[chuganzy]

Sorry if it is by design🙇🏻

[kcharwood]

Interesting. What is the best practice here? I would imagine that if you had an empty array, you wouldn't send the query parameter at all. Can you describe your use case?

It looks like we need to flush out some more tests out around this as well, as there isn't array specific tests for parameter encoding.

[cutz]

My opinion is that being able to distinguish between nil, not provided, and an empty array, provided but empty, seems like a useful thing to be able to communicate as part of an API interaction.

[kcharwood]

Any spec's out there that show best practices for this?

[cnoon]

We already have quite a few tests around most of the mentioned cases here @kcharwood in Alamofire. Maybe some of those could lend a hand here. Also, the public docstring for the URL case points out that there aren't any RFC specs published around query string collection types.

Here are a couple other issues with some additional details.

• Get request and array params Alamofire/Alamofire#329
• A number of parameters will be BUG？ Alamofire/Alamofire#484

[kcharwood]

That explains why I can't find anything :)

Making this change would be considered a breaking change IMO, since anyone using the current version of the library would get different behavior if they are currently passing in an empty array. In light of that, I would be hesitant to merge this in without strong consensus from the community that this should be the preferred way of dealing with an empty array.

[kcharwood]

We do need to get some tests around this though. I'd like to get that merged in.

[cutz]

I'm also not able to find anything. With respect to it being a breaking change, I agree.

[kcharwood]

@cnoon @kylef Any thoughts on where we should land for the empty array case?

[cnoon]

First off, if we did change this, it should only be changed with a MAJOR version bump and called out in the migration guide. I'd also say we should keep AFN and AF using the same approach.

With that said, I'm a bit torn on this. It seems fairly logical that a server would require a key/value pair as part of the contract. In the event that an array is empty, AFN and AF will not ever send that key/value pair with the current implementation. This seems like a shortcoming IMO. The problem is that there's not a great solution here. Passing nil is fairly misleading. The array isn't nil, it's empty. I'm not really sure whether it's more clear by sending nil, or more confusing.

I've literally written this response three different times with a different opinion each time. I'd say that I feel it should probably remain unchanged. I'm really interested to see what @kylef thinks.

[kcharwood]

My preference would be to leave the behavior as is.

If you need to provide custom serialization, you can do so by using setQueryStringSerializationWithBlock and creating your own custom query string serialization. Since we have the hook to do whatever custom processing you want, I'd err on keeping the existing behavior unless we see an overwhelming response that the current method is not acceptable, which we haven't seen yet.

[chuganzy]

ok, i see. thanks. may i close this PR?

[kcharwood]

Yes I'm going to close this one out.

Thanks!

🍻