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

[Vadim Zeitlin]

On Thu, 27 Sep 2018 12:53:50 -0400 (EDT) Greg Chicares <address@hidden> wrote:

GC> branch: master
GC> commit 58b0c04f9c50d6bdc1157793040641262192e5a0
GC> Author: Gregory W. Chicares <address@hidden>
GC> Commit: Gregory W. Chicares <address@hidden>
GC> 
GC>     Sort columns alphabetically in TSV output
GC>     
GC>     Incidentally, omit spaces between arguments in template instantiations
GC>     so that they're more easily recognizable as single syntactic units.
GC> ---
GC>  ledger_evaluator.cpp | 18 ++++++++++++++----
GC>  1 file changed, 14 insertions(+), 4 deletions(-)
GC> 
GC> diff --git a/ledger_evaluator.cpp b/ledger_evaluator.cpp
GC> index b609c00..a112a08 100644
GC> --- a/ledger_evaluator.cpp
GC> +++ b/ledger_evaluator.cpp
[...]
GC> @@ -1007,7 +1008,16 @@ ledger_evaluator Ledger::make_evaluator() const
GC>              );
GC>          fs::ofstream ofs(filepath, ios_out_trunc_binary());
GC>  
GC> -        for(auto const& j : stringvectors)
GC> +        // Copy 'stringvectors' to a (sorted) std::map in order to
GC> +        // show columns alphabetically. Other, more complicated
GC> +        // techniques are faster, but direct copying favors simplicity
GC> +        // over speed--appropriately, as this facility is rarely used.
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.

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

 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);

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

 Regards,
VZ