Re: [lmi] [lmi-commits] master 6335e9a 1/4: Render comprehensible

[Greg Chicares]

On 2017-01-26 16:55, Vadim Zeitlin wrote:
> On Thu, 26 Jan 2017 10:52:47 +0000 Greg Chicares <address@hidden> wrote:
[...]
> GC> I'm working on this now and will push something soon for your review.
> 
>  If this something was 60341554d995fc700e599b59a00ddee6c2882361, then I can

Yes, I pushed that especially for this reason.

> only say that I'm perfectly happy with it, there is no duplication any more
> and I prefer seeing check() to check0().
> 
>  Two very minor comments concern default arguments:
> 
> 1. There is no need for them at all in analyze_errors(), as it's always
>    called with the full set, so why specify them there?

I was actually surprised to discover that no such function should
ever have existed in the first place. I guess it was my attempt to
factor out whatever was common to check0(), check1(), and check2(),
which should never have been distinct functions either.

Finally I have a unit test module that I think I can understand,
so I can soon begin the real work of cleaning up the class it tests.

> 2. Why use "std::vector<std::string>(0)" instead of, IMO more clear,
>    "std::vector<std::string>()"?
> 
>  Sorry for nitpicking,

There's no such thing as nitpicking in code review IMO, and I'll
certainly get rid of that "0", thanks. But first I'll have to search
lmi for similar silliness that can be removed at the same time...