On Wed, May 03, 2017 at 10:47:45AM +0200, Andreas Reichel wrote: > On Tue, May 02, 2017 at 04:36:22PM +0300, Ed Bartosh wrote: > > On Fri, Apr 21, 2017 at 02:11:45PM +0200, Andreas J. Reichel wrote: > > > To increase code maintainability, use dictionaries > > > as enum-like container for parameter string comparisons. > > > > > > Signed-off-by: Andreas Reichel <[email protected]> > > > Signed-off-by: Jan Kiszka <[email protected]> > > > Signed-off-by: Daniel Wagner <[email protected]> > > > > > > --- > > > scripts/lib/wic/engine.py | 16 +++++++++++++--- > > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > > > diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py > > > index 647358287f..1428a73ba8 100644 > > > --- a/scripts/lib/wic/engine.py > > > +++ b/scripts/lib/wic/engine.py > > > @@ -35,6 +35,16 @@ from wic import WicError > > > from wic.pluginbase import PluginMgr > > > from wic.utils.misc import get_bitbake_var > > > > > > +class StrEnum(dict): > > > + __getattr__ = dict.get > > > + > > > +ListType = StrEnum({ > > > + 'LIST_IMAGES': "images", > > > + 'LIST_SRC_PLUGINS': "source-plugins" }) > > > + > > > +HelpArg = StrEnum({ > > > + 'HELP': "help" }) > > > + > > > logger = logging.getLogger('wic') > > > > > > def verify_build_env(): > > > @@ -204,14 +214,14 @@ def wic_list(args, scripts_path): > > > if args.list_type is None: > > > return False > > > > > > - if args.list_type == "images": > > > + if args.list_type == ListType.LIST_IMAGES: > > > > > > list_canned_images(scripts_path) > > > return True > > > - elif args.list_type == "source-plugins": > > > + elif args.list_type == ListType.LIST_SRC_PLUGINS: > > > list_source_plugins() > > > return True > > > - elif len(args.help_for) == 1 and args.help_for[0] == 'help': > > > + elif len(args.help_for) == 1 and args.help_for[0] == HelpArg.HELP: > > > wks_file = args.list_type > > > fullpath = find_canned_image(scripts_path, wks_file) > > > if not fullpath: > > > > I'm not sure if this increases maintainability, but it definitely increases > > code complexity and > > decreases readability. Can you explain the idea in a bit more detailed way? > > > In general, if string constants are used as keys, it is better do define them > as > constants. Maintainability increases because if you have to change them, > you only change them in one place. You have a central place for this > definition and not scattered them all over the code. That's the idea. >
Not sure I'm convinced. From my point of view this patch changes quite simple and readable code to something more complex and less readable for no visible reason. If I have to change 'images' or 'source-plugins'(why would I want to do that?) I'd just go and change them in one place as they're not used anywhere else. -- Regards, Ed -- _______________________________________________ Openembedded-core mailing list [email protected] http://lists.openembedded.org/mailman/listinfo/openembedded-core
