Re: [lmi] [lmi-commits] master 58b0c04 3/9: Sort columns alphabetically in TSV output

[Greg Chicares]

On 2018-09-27 20:37, Vadim Zeitlin wrote:
> On Thu, 27 Sep 2018 12:53:50 -0400 (EDT) Greg Chicares <address@hidden> wrote:
[...]
> GC> +        std::map<std::string,std::vector<std::string>> 
> ordered_stringvectors
> GC> +            (stringvectors.begin()
> GC> +            ,stringvectors.end()
> GC> +            );
> 
>  That's another nitpick, but shouldn't be the parentheses above be braces,
> i.e.
> 
>       std::map<std::string,std::vector<std::string>> ordered_stringvectors
>           {stringvectors.begin()
>           ,stringvectors.end()
>           };
> 
> ? I don't think we can get into trouble with the initializer list ctor
> overload here, as an iterator shouldn't be convertible to a pair (I hope?),
> but please let me know if I'm missing anything.

Our discussions on this topic are still evolving, and there's room for
them to evolve further; but IIRC the closest we've come to a rule is
to use () when we're "calling" a special member function, as in
  std::vector<int> v(10, 7); // Calls vector::vector(int len, T value).
and to use {} when we're "moving" or "copying" a value, e.g.:
  std::vector<int> v2 {v1};  // Calls vector::vector(vector const&).

In this case, I'd say we're calling a ctor:
  map::map(iterator, iterator);
An iterator isn't a map, so we aren't "moving" or "copying" its
arguments; we're calling the ctor with those arguments, so it's more
like a function call IMO.

Another way to put this is: use {} for operations that a fairly simple
compiler could be expected to optimize away, and () for operations
that require real work to be done (unless the optimizer is very
sophisticated). Here, I expect that at runtime the hashmap will be
traversed and an RB tree will be constructed on the fly.

A stronger criterion (which I'm not entirely convinced is appropriate)
would be to use {} iff assignment makes the LHS and RHS identical in
the sense that if
  T lhs = {rhs};
then there's no way to write a function f<T>() that gives different
results for f(lhs) and f(rhs). Of course that wouldn't be satisfied
here because std::map and std::unordered_map are different types.

IIUC, you're suggesting a different rule: use {} unless it would be
semantically incorrect--i.e., almost always. I'm not sure what
"almost always" means; I only know that std::vector(count, value)
is one exception, but I don't know what other exceptions may be
necessary.

>  And, while I'm nitpicking, shouldn't the ordered_stringvectors be const?

Agreed: commit 5e45c3d48.

>  Also, I don't mind sorting unordered map in such a way, but I think that
> if we need to do it just one more time, it would be worth adding some
> make_sorted() template function with the trivial implementation that could
> be used to write just
> 
>       auto const ordered_stringvectors = make_sorted(stringvectors);

That might convert unordered containers to their sorted analogues.
We could alternatively consider a bidirectional converter; or
maybe a general converter between "compatible" containers, but
perhaps there's no use case for, e.g., converting a list to a deque.

> instead of having to write all the 46 characters of the map type
> explicitly.

00000000011111111112222222222333333333344444444445555555555666666666677777777778
12345678901234567890123456789012345678901234567890123456789012345678901234567890
std::map<std::string,std::vector<std::string>> ordered_stringvectors

At least I didn't use std::basic_string<...>.