On 07/18/2016 06:15 AM, Pavel Grunt wrote:
> Hi Eduardo,
>
> On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote:
>> Use switch/case instead of lots of conditional blocks
> Yes, it is more readable
>>
>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx>
>> ---
>> src/ovirt-foreign-menu.c | 76 +++++++++++++++++++++++----------------------
>> ---
>> 1 file changed, 36 insertions(+), 40 deletions(-)
>>
>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
>> index 33ff4f1..b0b8fec 100644
>> --- a/src/ovirt-foreign-menu.c
>> +++ b/src/ovirt-foreign-menu.c
>> @@ -312,51 +312,47 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu
>> *menu,
>> g_return_if_fail(current_state >= STATE_0);
>> g_return_if_fail(current_state < STATE_ISOS);
>>
>> - current_state++;
> my preference is to keep the increment outside the switch statement
>
>> -
>> - if (current_state == STATE_API) {
>> - if (menu->priv->api == NULL) {
>> - ovirt_foreign_menu_fetch_api_async(menu);
>> - } else {
>> - current_state++;
>> + switch (++current_state) {
> Actually the increment is not needed at all thanks to your changes, imo
> switch(current_state + 1) would be more readable
Alright, I don't mind at all.
>> + case STATE_API: {
> 'case' should have the same indentation as its 'switch'
>
> Remove extra {}, no need to have the null check in the extra block (applies to
> all cases)
>
I don't think so, the if checks are necessary for the initialization
process, when we have everything initalized, it will fall straight to
the STATE_ISOS case. Or maybe you are talking about something else and I
misunderstood?
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko@xxxxxxxxxx
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
