Re: [wic patch 5/5] wic: Use enum like dicts for string constants

[Andreas Reichel <Andreas.Reichel@tngtech.com>]

[-- Attachment #1: message --]
[-- Type: text/plain, Size: 2816 bytes --]

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 <andreas.reichel.ext@siemens.com>
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com>
> > 
> > ---
> >  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.

> --
> Regards,
> Ed

-- 
Andreas Reichel 
Dipl.-Phys. (Univ.) 
Software Consultant

Andreas.Reichel@tngtech.com 
+49-174-3180074

TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterföhring
Geschäftsführer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke
Sitz: Unterföhring * Amtsgericht München * HRB 135082

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 849 bytes --]