Make reverse more DWIM-y

[davidfetter]

Strings and numbers go backward.
Booleans flip.
Objects switch keys and values.

[coveralls]

Coverage remained the same at 84.457% when pulling 0b5da6e on davidfetter:expand_reverse into 0bc7708 on stedolan:master.

[leonid-s-usov]

Reversing strings totally makes sense!

Boolean - it's weird. I'd not include it though, you have not to reverse bools in a predictable and typesafe way.

Numbers just don't make sense. If that's really what you want, I'd rather that you make it explicit in your code .number | tostring | reverse. The issue is that reversing an arbitrary valid number's string representation will probably give you an error trying to convert it back. Consider -1, 3e+3. And in any case, it's pretty silly unless it's a very specific requirement which doesn't deserve a builtin

[pkoppstein]

@davidfetter - Some observations:

a) there is no reason to define $in in the body of the def;

b) for strings, explode | reverse | implode is much faster (approx 2 times according to my measurements) and also significantly more economical with memory than using split/join;

c) In my opinion, the proposed def is inappropriate for objects as the proposed operation is a kind of inversion or transposition. A more appropriate def (perhaps even the DWIM definition) would reverse the order of keys. In addition, this def makes it very easy for users to ignore all the issues that key/value inversion can present. If someone wants to invert keys and values in some fashion, they should be aware of all the issues, and be responsible for their resolution.

d) I would go even further than @leonid-s-usov and argue that the domain of reverse should not be extended beyond arrays and strings, mainly to avoid bloat and duplication of function. The simplicity of explode|reverse|implode could even be taken as an argument against the wisdom of an extension to strings :-)

[leonid-s-usov]

Yeah.. I’d say that anything “explodable” should be also “reversible” If one defines explode for objects to be to_entries then yeah, reverse would change the order of keys. Sent from mobile device, typos and false corrections are possible Regards, Leonid S. Usov

…

On 25 Oct 2018, at 11:57, pkoppstein ***@***.***> wrote: @davidfetter - Some observations: a) there is no reason to define $in in the body of the def; b) for strings, explode | reverse | implode is much faster (approx 2 times according to my measurements) and also significantly more economical with memory than using split/join; c) In my opinion, the proposed def is inappropriate for objects as the proposed operation is a kind of inversion or transposition. A more appropriate def (perhaps even the DWIM definition) would reverse the order of keys. In addition, this def makes it very easy for users to ignore all the issues that key/value inversion can present. If someone wants to invert keys and values in some fashion, they should be aware of all the issues, and be responsible for their resolution. d) I would go even further than @leonid-s-usov and argue that the domain of reverse should not be extended beyond arrays and strings, mainly to avoid bloat and duplication of function. The simplicity of explode|reverse|implode could even be taken as an argument against the wisdom of an extension to strings :-) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[davidfetter]

I've trimmed the extension back just to strings and removed fat from the code.

The behavior for other types is compatible with the current behavior.

[pkoppstein]

@davidfetter - The trimmed down def has three bugs in it (e.g. it still assumes $in has been defined).

Also, to avoid the second type check, I'd suggest:

def reverse:
  def r: [.[length - 1 - range(0;length)]];
  if type == "string" then explode | r | implode else r end;

If you wanted to propose the behavior on JSON objects that reverses the order of keys:

def reverse:
  def r: [.[length - 1 - range(0;length)]];
  if type == "string" then explode | r | implode
  elif type == "object" then to_entries | r | from_entries
  else r
  end;

By the way, please note that it will be up to the maintainers to decide whether and when to include your contribution.

[davidfetter]

I really appreciate your patient help here.

[nicowilliams]

Sure, why not.

[itchyny]

Related issue: #412.