lo.Mode and it.Mode inconsistencies

[NathanBaulch]

#709 and #711 introduce inconsistencies that I'd like to address.

1. Whilst implementing it.Mode I noticed that a new slice was being allocated (mode = []T{item}) each time a larger frequency is found. I addressed this by reusing the original slice (mode = append(mode[:0], item)). The issue from @d-enk recommended this change be applied to the lo.Mode slice version but the PR from @intojhanurag did not make this change.
2. If truncating the capacity of return slices is deemed important (questionable!) then why would we apply it to this particular helper and not the 48 other helpers that return slices?
3. There is no advantage to using the more verbose make([]T, 0) syntax that I'm aware of. See lint: use consistent short slice creation syntax #729.

My preference is to roll back #711 and replace with the simple slice reuse fix mentioned in item 1 above.

[samber]

Thanks @NathanBaulch for pointing out this issue.

I agree with you, this project needs a strong consistency for:

• core package and sub-packages
• a predictable policy about the capacity of returned slices
• common code style

I consider that a developer using lo is looking for productivity. Those who want max performance should use their own implementation of lo helpers.

So I would opt for a truncated capacity for every helper, because it offers a predictable output, even if it implies a little more processing.

WDYT?

Note:

• we need more tests on capacity
• all these choices should be enforced in contributor guidelines

I am currently testing AI reviews in samber/ro. As soon as it's mature, I will add it to samber/lo, and such guidelines will ease the detection of inconsistencies.

[intojhanurag]

Hey @NathanBaulch , Thanks for the review .

[NathanBaulch]

I've never encountered a perf issue that was solved by truncating the capacity of slices, but I guess it's possible in certain circumstances.

I personally don't think we should since:

1. truncating capacity alone does not make the surplus memory GC eligible - you still need to do a copy.
2. consumers that append to slices returned by our helpers will see degraded perf - no longer any spare space.
3. admittedly negligible but truncating capacity has a small cost - both runtime perf and dev maintenance (enforcing this pattern).

[d-enk]

In fact, first of all, I was worried that there might be garbage (not zero values) left in the mode[len(mode):cap(mode)]

So, it is possible to use something like clear(mode[len(mode):len(mode)])

But perhaps we can leave it alone...