On Wed, 2017-10-04 at 19:39 +0300, Luciano Coelho wrote:
> On Wed, 2017-10-04 at 09:26 -0700, Joe Perches wrote:
[]
> > This might be more intelligble as separate tests
> >
> > static bool is_valid_channel(u16 ch_id)
> > {
> > if (ch_id <= 14)
> > return true;
> >
> > if ((ch_id % 4 == 0) &&
> > ((ch_id >= 36 && ch_id <= 64) ||
> > (ch_id >= 100 && ch_id <= 140)))
> > return true;
> >
> > if ((ch_id % 4 == 1) &&
> > (chid >= 145 && ch_id <= 165))
> > return true;
> >
> > return false;
> > }
> >
> > The compiler should produce the same object code.
>
> Yeah, it may be a bit easier to read, but I don't want to start getting
> "fixes" to working and reasonable code. There's nothing wrong with the
> existing function (except maybe for the int vs. boolean) so let's not
> change it.
>
> A good time to change this would be the next time someone adds yet
> another range of valid channels here. ;)
<shrug> Your choice.
I like code I can read and understand at a glance.
At case somebody needs to add channels, likely nobody
would do the change suggested but would just add
another test to the already odd looking block.
And constants should be on the right side of the tests.
