query parser status

[Ted Dunning <te...@gmail.com>]

I have checked in a rearranged query parser just now.

There is a lot of work to get this up to acceptable code quality.  Some
quick notes:

- the expression parser section of the grammar uses literally dozens of
productions named with a single letter and a number.  This isn't good.

- the tests don't use proper junit style.  Everything is assertTrue even
when checking for equality or null.  This is stylistic, but bad style
smells bad.  I have corrected about 70% of these in one test file.  There
are lots more.

- the tests don't work because of impossible casts.  I don't think that
this is the result of moving the code around because I don't think that
these tests could every have worked.

- there are unused imports and variables all over.  This indicates a lack
of dilligence and should be fixed.

- lots of methods in test classes are called test* but they aren't tests.
 This is misleading to the reader.


Camuel,

Can you goad somebody in your shop into fixing these issues?

Re: query parser status

[Camuel Gilyadov <ca...@gmail.com>]

yep, will take care of it.

On Mon, Oct 22, 2012 at 2:49 PM, Ted Dunning <te...@gmail.com> wrote:

> I have checked in a rearranged query parser just now.
>
> There is a lot of work to get this up to acceptable code quality.  Some
> quick notes:
>
> - the expression parser section of the grammar uses literally dozens of
> productions named with a single letter and a number.  This isn't good.
>
> - the tests don't use proper junit style.  Everything is assertTrue even
> when checking for equality or null.  This is stylistic, but bad style
> smells bad.  I have corrected about 70% of these in one test file.  There
> are lots more.
>
> - the tests don't work because of impossible casts.  I don't think that
> this is the result of moving the code around because I don't think that
> these tests could every have worked.
>
> - there are unused imports and variables all over.  This indicates a lack
> of dilligence and should be fixed.
>
> - lots of methods in test classes are called test* but they aren't tests.
>  This is misleading to the reader.
>
>
> Camuel,
>
> Can you goad somebody in your shop into fixing these issues?
>