[virt-tools-list] [PATCH virt-viewer 01/11] ovirt-foreign-menu: Rework states logic

[Christophe Fergeau]

On Mon, Jul 18, 2016 at 11:15:03AM +0200, 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 at 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

I'd go for that too as this commit removes all the other current_state++
occurrences.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20160719/cd6d7bdb/attachment.sig>