> On 20.03.2018 12:35, Weglicki, MichalX wrote: > >> -----Original Message----- > >> From: Ilya Maximets [mailto:i.maxim...@samsung.com] > >> Sent: Tuesday, March 20, 2018 9:50 AM > >> To: Weglicki, MichalX <michalx.wegli...@intel.com>; > >> ovs-dev@openvswitch.org > >> Cc: Heetae Ahn <heetae82....@samsung.com>; Ben Pfaff <b...@ovn.org>; > >> Stokes, Ian <ian.sto...@intel.com> > >> Subject: Re: [PATCH] netdev-dpdk: Refactor custom stats. > >> > >> On 19.03.2018 13:22, Weglicki, MichalX wrote: > >>> Hello, > >> > >> Hello. > >> > >>> > >>> I've went through the patch quite carefully. > >> > >> Thanks for reviewing this. > >> > >>> Main change refactors creation cached IDs and Names from IF-like code > block to "Goto" code block. > >> > >> Current code is over-nested. It has nesting level of 6 (7 including > function definition). > >> It's like: > >> netdev_dpdk_configure_xstats > >> { > >> if () { > >> if () { > >> ... > >> } else { > >> ... > >> if () { > >> ... > >> if () { > >> } else if { > >> } > >> ... > >> if () { > >> for () { > >> if () { <-- level #7 ! > >> } > >> } > >> } else { > >> } > >> } > >> } > >> } else { > >> } > >> if () { > >> } > >> } > >> > >> > >> IMHO, This is not readable. Especially, combining with constant line > >> wraps because of limited line lengths. I wanted to make plain > >> execution sequence to simplify understanding of the code. Most of the > 'if' statements are just error checking. > >> And the single exit point from such conditions usually implemented by > 'goto's. > >> It's a common practice for system programming. It doesn't worth to > >> keep so deep nesting level for error checking conditions. This also > >> matches the style of all the other code in netdev-dpdk and not only > here. > > > > For me it is straightforward error checking, so I don't really see an > advantage. > > Not everything is implemented using "goto" in netdev-dpdk. I assume > > this is preference thing so maybe we could ask someone to make a call, > Ben/Ian? > > > Would like to hear some opinions too. >
I was familiar with the existing code but after comparing the two approaches, from a purely aesthetic point the goto solution looks cleaner and a little simpler to understand on first pass. As examples of both approaches are present in netdev-dpdk this comes to preference, I'd be in favor the new goto format being introduced once all error cases are accounted for. Ian _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
