Re: [PATCH 18/22] ui_out_table: Replace boolean flag with enum

Pedro Alves
2016-11-30T12:10:56Z

On 11/26/2016 04:47 PM, Simon Marchi wrote:
> On 2016-11-24 13:42, Pedro Alves wrote:
>> On 11/24/2016 03:27 PM, Simon Marchi wrote:
>>> This patch is just a little cleanup, it replaces the body_flag field of
>>> ui_out_table with an enum. It expresses more explicitly the
>>> intent of the field (check that state == TABLE_STATE_HEADERS conveys
>>> more what we want to do than checking for !body_flag).
>>
>> Yay for avoiding the boolean trap.
>>
>>
>>> @@ -271,7 +279,7 @@ ui_out_table_begin (struct ui_out *uiout, int
>>> nbrofcols,
>>> previous table_end."));
>>>
>>> uiout->table.flag = 1;
>>> - uiout->table.body_flag = 0;
>>> + uiout->table.state = ui_out_table_state::TABLE_STATE_HEADERS;
>>
>> Nit: This one stood out, as none of the other places fully qualify
>> the enum.
>>
>> You could also consider moving the enum to within
>> the table class, and/and use "enum class" to shorten the
>> names, if you want to scope it.
>
> TIL about "enum class".
>
> In patch "Class-ify ui_out_table", I move it to the ui_out_table class.
> If that's ok with you, I'll look into making use of enum class in that
> patch.

Ack, I'll go look at it.

> I'll remove the "ui_out_table_state::" in this instance for
> consistency.

Thanks. This patch LGTM with that.

--
Pedro Alves