jq icon indicating copy to clipboard operation
jq copied to clipboard

Make reverse more DWIM-y

Open davidfetter opened this issue 7 years ago • 9 comments

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

davidfetter avatar Oct 25 '18 05:10 davidfetter

Coverage Status

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

coveralls avatar Oct 25 '18 05:10 coveralls

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

leonid-s-usov avatar Oct 25 '18 07:10 leonid-s-usov

@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 :-)

pkoppstein avatar Oct 25 '18 08:10 pkoppstein

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 [email protected] 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.

leonid-s-usov avatar Oct 25 '18 10:10 leonid-s-usov

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.

davidfetter avatar Oct 29 '18 04:10 davidfetter

@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.

pkoppstein avatar Oct 29 '18 04:10 pkoppstein

I really appreciate your patient help here.

davidfetter avatar Oct 29 '18 14:10 davidfetter

Sure, why not.

nicowilliams avatar Jan 02 '19 21:01 nicowilliams

Related issue: #412.

itchyny avatar Jun 25 '23 04:06 itchyny