You are viewing a plain text version of this content. The canonical link for it is here.
Posted to adffaces-dev@incubator.apache.org by Simon Lessard <si...@gmail.com> on 2007/03/16 01:25:26 UTC

Getting rid of an ugly assert usage

Hello all,

I would like to get rid of the following code snippet from
org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.table.RowData and
make that usage forbidden in our coding conventions wiki, as it's really a
poor man's #ifdef __DEBUG and goes against the idea of assert not having any
performance overhaul at runtime.

    boolean assertEnabled = false;
    assert assertEnabled = true;

    ...
    if (assertEnabled)
    {
      // make sure prev operation was get:
      assert (_currentRowSpanState == 2);
      _currentRowSpanState = 0; // indicate that we have reset the rowspan
    }

Any objection?


~ Simon

Re: Getting rid of an ugly assert usage

Posted by Jeanne Waldman <je...@oracle.com>.
That's pretty tricky... and confusing!
I'm fine with getting rid of it.

- Jeanne

Simon Lessard wrote:
> Well yeah, it's very hard to read
>
> boolean assertEnabled = false; // Ok we got a variable with value set to
> false
>
> assert assertEnabled = true; // This is an assignment, not a 
> comparison, so
> if assert is enabled, the line will be executed and assertEnabled 
> value will
> now be true. Furthermore, since the value of an assignment is the 
> assigned
> value, the assert is really assert true; and thus never fail. If 
> assert is
> not enabled, assertEnabled keeps its value to false.
>
> if (assertEnabled)
> {
>  // This code will execute only if assert is enabled
> }
>
>
> It's really really ugly imho.
>
> On 3/15/07, Jeanne Waldman <je...@oracle.com> wrote:
>>
>> You mean this part?
>>     boolean assertEnabled = false;
>>     assert assertEnabled = true;
>>
>>     ...
>>     if (assertEnabled)
>>
>>
>> It doesn't make sense to me.
>>
>> - Jeanne
>>
>> Adam Winer wrote:
>> > Yeah, that abuse of assert always bugged me.
>> >
>> > -- Adam
>> >
>> >
>> > On 3/15/07, Simon Lessard <si...@gmail.com> wrote:
>> >>
>> >> Hello all,
>> >>
>> >> I would like to get rid of the following code snippet from
>> >> 
>> org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.table.RowData
>> >> and
>> >> make that usage forbidden in our coding conventions wiki, as it's
>> >> really a
>> >> poor man's #ifdef __DEBUG and goes against the idea of assert not
>> having
>> >> any
>> >> performance overhaul at runtime.
>> >>
>> >>     boolean assertEnabled = false;
>> >>     assert assertEnabled = true;
>> >>
>> >>     ...
>> >>     if (assertEnabled)
>> >>     {
>> >>       // make sure prev operation was get:
>> >>       assert (_currentRowSpanState == 2);
>> >>       _currentRowSpanState = 0; // indicate that we have reset the
>> >> rowspan
>> >>     }
>> >>
>> >> Any objection?
>> >>
>> >>
>> >> ~ Simon
>> >>
>> >
>>
>

Re: Getting rid of an ugly assert usage

Posted by Simon Lessard <si...@gmail.com>.
Well yeah, it's very hard to read

boolean assertEnabled = false; // Ok we got a variable with value set to
false

assert assertEnabled = true; // This is an assignment, not a comparison, so
if assert is enabled, the line will be executed and assertEnabled value will
now be true. Furthermore, since the value of an assignment is the assigned
value, the assert is really assert true; and thus never fail. If assert is
not enabled, assertEnabled keeps its value to false.

if (assertEnabled)
{
  // This code will execute only if assert is enabled
}


It's really really ugly imho.

On 3/15/07, Jeanne Waldman <je...@oracle.com> wrote:
>
> You mean this part?
>     boolean assertEnabled = false;
>     assert assertEnabled = true;
>
>     ...
>     if (assertEnabled)
>
>
> It doesn't make sense to me.
>
> - Jeanne
>
> Adam Winer wrote:
> > Yeah, that abuse of assert always bugged me.
> >
> > -- Adam
> >
> >
> > On 3/15/07, Simon Lessard <si...@gmail.com> wrote:
> >>
> >> Hello all,
> >>
> >> I would like to get rid of the following code snippet from
> >> org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.table.RowData
> >> and
> >> make that usage forbidden in our coding conventions wiki, as it's
> >> really a
> >> poor man's #ifdef __DEBUG and goes against the idea of assert not
> having
> >> any
> >> performance overhaul at runtime.
> >>
> >>     boolean assertEnabled = false;
> >>     assert assertEnabled = true;
> >>
> >>     ...
> >>     if (assertEnabled)
> >>     {
> >>       // make sure prev operation was get:
> >>       assert (_currentRowSpanState == 2);
> >>       _currentRowSpanState = 0; // indicate that we have reset the
> >> rowspan
> >>     }
> >>
> >> Any objection?
> >>
> >>
> >> ~ Simon
> >>
> >
>

Re: Getting rid of an ugly assert usage

Posted by Jeanne Waldman <je...@oracle.com>.
You mean this part?
    boolean assertEnabled = false;
    assert assertEnabled = true;

    ...
    if (assertEnabled)


It doesn't make sense to me.

- Jeanne

Adam Winer wrote:
> Yeah, that abuse of assert always bugged me.
>
> -- Adam
>
>
> On 3/15/07, Simon Lessard <si...@gmail.com> wrote:
>>
>> Hello all,
>>
>> I would like to get rid of the following code snippet from
>> org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.table.RowData 
>> and
>> make that usage forbidden in our coding conventions wiki, as it's 
>> really a
>> poor man's #ifdef __DEBUG and goes against the idea of assert not having
>> any
>> performance overhaul at runtime.
>>
>>     boolean assertEnabled = false;
>>     assert assertEnabled = true;
>>
>>     ...
>>     if (assertEnabled)
>>     {
>>       // make sure prev operation was get:
>>       assert (_currentRowSpanState == 2);
>>       _currentRowSpanState = 0; // indicate that we have reset the 
>> rowspan
>>     }
>>
>> Any objection?
>>
>>
>> ~ Simon
>>
>

Re: Getting rid of an ugly assert usage

Posted by Adam Winer <aw...@gmail.com>.
Yeah, that abuse of assert always bugged me.

-- Adam


On 3/15/07, Simon Lessard <si...@gmail.com> wrote:
>
> Hello all,
>
> I would like to get rid of the following code snippet from
> org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.table.RowData and
> make that usage forbidden in our coding conventions wiki, as it's really a
> poor man's #ifdef __DEBUG and goes against the idea of assert not having
> any
> performance overhaul at runtime.
>
>     boolean assertEnabled = false;
>     assert assertEnabled = true;
>
>     ...
>     if (assertEnabled)
>     {
>       // make sure prev operation was get:
>       assert (_currentRowSpanState == 2);
>       _currentRowSpanState = 0; // indicate that we have reset the rowspan
>     }
>
> Any objection?
>
>
> ~ Simon
>