query-string->coll and vice versa

[velios]

After the discussion in #47, I realized that my proposal was probably too complex and it would be better to break it down into several parts. This PR assumes a basic implementation without additionally features like into and keywordizе. Only the functionality itself, without DRY optimizations, etc.

[velios]

(query-string->coll "a=1&b=2") ;; => [["a" 1] ["b" 2]]
(coll->query-string [["a" 1] ["b" 2]]) ;; => "a=1&b=2"

(def query-coll-with-keywords (into [] (sort {:b 2 :a 1})) ;; => [[:a 1] [:b 2]]
(coll->query-string query-coll-with-keywords) ;; => also "a=1&b=2"

All points mention here is also valid for this PR

[plexus]

This looks reasonable. Lets change coll to vec in the function names, add docstrings, and add a CHANGELOG entry, and this should be good to go.

[velios]

@plexus Great. I'll take care of it. I didn't call it a (query-string->vec ...) because of (coll->query-string ...) expresses intention better. If i do (query-string->vec ...) & (coll->query-string ...) it can be confusing. Also, if we decide to use the into parameter by analogy with functions for maps, then the naming coll will be more appropriate. What do you think about it?

[velios]

I have made all the edits you indicated. I don't think it's a good solution change coll to vec, but this is too minor a detail for me, so I did as recommended.

[plexus]

Does the return value have to be a vector? Why not drop the (vec) and call it query-params-seq, and the opposite query-params-string? coll->query-string isn't accurate because it takes any seq(able), not just concrete collections.

You should also drop the explicit (seq) call, map already calls seq.

[plexus]

I would use the word "positional" in the docstring, in my mind that's the essence of these functions. If you want to deal with params in a specific positional way, rather than in an associative way.

[velios]

I use vector instead of just leave because prefer a calculated result to all lazy evaluations unless it is clear what the benefits of being lazy are. If you think differently, we can do it your way. In this case it's not a big deal. And i don't understand what return instead of default vector/

I use (seq q) besause of this case of usage (query-string->seq ""):

(->> (str/split "" #"&")
     (map decode-param-pair))
=> (["" ""])

You can simple get "" result, when parse this url

(:query (uri "http://www.example.com/?")) => ""

And if you try to revert it you will have unexpected result

(some->> (seq [["" ""]])
         (map (fn [[k v]] (encode-param-pair k v)))
         (interpose "&")
         (apply str))
=> "="

And it led to

(assoc (uri "https://example.com") :query "=")
=> #lambdaisland/uri"https://example.com?="

It's unexpected and maybe led to errors

So i try to avoid this situation and when empty string pass to (query-string->seq "") it will return empty seq.

At the moment I can't figure out how to rewrite the code so that it fulfills your wishes. Could you give a little more hints?

[velios]

I also agree with any edits to the docstring. I'm not a native English speaker, so I can't always come up with the most expressive option.

[velios]

I made corrections, having tried to take into account all the comments in the best way. I still think that returning the vector is the best solution from the practical side of the case. Write what other corrections will be required?

(sequential? []) ;; => true

[velios]

@plexus What can I do to speed up the review?

[velios]

It seems to me that we will never be able to push this small and essentially harmless change. I have enormous respect for other people's time, but it is also a completed PR.

[plexus]

Released in v1.17.141

[lambdaisland/uri "1.17.141"]                 ;; deps.edn
{lambdaisland/uri {:mvn/version "1.17.141"}}  ;; project.clj

[plexus]

Thanks for the patch!

[velios]

I apologize that my commits have meaningless description in the main branch, I will be more careful in the next PR. I assumed they would be added to the code via squash and merge github option.