[PATCH] Add a "sort role" for sorting the dive list

Dirk Hohndel dirk at hohndel.org
Wed May 29 14:23:36 PDT 2013


On Thu, 2013-05-30 at 06:19 +0900, Linus Torvalds wrote:
> On Thu, May 30, 2013 at 6:06 AM, Dirk Hohndel <dirk at hohndel.org> wrote:
> >
> > Wait one minute and pull.
> >
> > It is ALL WORKING. Beautifully.
> 
> Well, almost. The sorting works again, but the sorting marker doesn't
> show up. So you don't actually visually see which column things are
> sorted by any more.
> 
> On a code style issue, I have to say that the
> 
>   switch (type) {
>    case X: ret = ... ; break;
>    }
>    return ret;
> 
> coding style is just retarded. The comment says "makes debugging a
> little easier" which I don't understand at all. 

I somewhat frequently end up wanting to see what a function like this
returns. And that can be quite easily done with a few qDebug() at the
end of it, right before the return.

> It just makes the code
> harder to read when you have to look at the very end of the line (that
> is so long that it may not even fit in your window) to know if a case
> statement breaks out or not.

I would NEVER do that if some cases fall through and others don't.
This only makes sense if EVERY line ends on a break; and that's easy to
check with one glance.

So let's agree to disagree here.

/D



More information about the subsurface mailing list