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) <etru...@redhat.com> > --- > 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 > + 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) Pavel > + if (menu->priv->api == NULL) { > + ovirt_foreign_menu_fetch_api_async(menu); > + break; > + } > } > - } > - > - if (current_state == STATE_VM) { > - if (menu->priv->vm == NULL) { > - ovirt_foreign_menu_fetch_vm_async(menu); > - } else { > - current_state++; > + case STATE_VM: { > + if (menu->priv->vm == NULL) { > + ovirt_foreign_menu_fetch_vm_async(menu); > + break; > + } > } > - } > - > - if (current_state == STATE_STORAGE_DOMAIN) { > - if (menu->priv->files == NULL) { > - ovirt_foreign_menu_fetch_storage_domain_async(menu); > - } else { > - current_state++; > + case STATE_STORAGE_DOMAIN: { > + if (menu->priv->files == NULL) { > + ovirt_foreign_menu_fetch_storage_domain_async(menu); > + break; > + } > } > - } > - > - if (current_state == STATE_VM_CDROM) { > - if (menu->priv->cdrom == NULL) { > - ovirt_foreign_menu_fetch_vm_cdrom_async(menu); > - } else { > - current_state++; > + case STATE_VM_CDROM: { > + if (menu->priv->cdrom == NULL) { > + ovirt_foreign_menu_fetch_vm_cdrom_async(menu); > + break; > + } > } > - } > - > - if (current_state == STATE_CDROM_FILE) { > - ovirt_foreign_menu_refresh_cdrom_file_async(menu); > - } > - > - if (current_state == STATE_ISOS) { > - g_warn_if_fail(menu->priv->api != NULL); > - g_warn_if_fail(menu->priv->vm != NULL); > - g_warn_if_fail(menu->priv->files != NULL); > - g_warn_if_fail(menu->priv->cdrom != NULL); > + case STATE_CDROM_FILE: { > + ovirt_foreign_menu_refresh_cdrom_file_async(menu); > + break; > + } > + case STATE_ISOS: { > + g_warn_if_fail(menu->priv->api != NULL); > + g_warn_if_fail(menu->priv->vm != NULL); > + g_warn_if_fail(menu->priv->files != NULL); > + g_warn_if_fail(menu->priv->cdrom != NULL); > > - ovirt_foreign_menu_refresh_iso_list(menu); > + ovirt_foreign_menu_refresh_iso_list(menu); > + break; > + } > + default: { > + g_warn_if_reached(); > + } > } > } > _______________________________________________ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list