Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Useless __initialized__ stuff #55

Closed
sanguinariojoe opened this issue Jun 4, 2015 · 3 comments
Closed

Useless __initialized__ stuff #55

sanguinariojoe opened this issue Jun 4, 2015 · 3 comments

Comments

@sanguinariojoe
Copy link

It is a little bit strange how FoBiS is initialized, resulting in an unnecessary obfuscation...

In fobis.py:7, you are importing the configuration module (which is actually FoBiS itself, or at least the main engine) with a strange command:

from .config import __config__

It is used just when you are trying to generate a private subclass, but I can't see the point here. Anyway... This command is executing the code in config.py:642:

# global variables
__initialized__ = False
if not __initialized__:
  __config__ = FoBiSConfig()
  __initialized__ = True

Which, as far as I can see, is trying to avoid calling FoBiSConfig creation more than once. I should mention right here that just init() has been defined in the class, no new(). Then program is coming back to fobis.py:14:

__config__.run_fobis()

In the very first line of the function the following is executed (config.py:612):

self._reset()

Which are just calling:

self.__init__()

So, an initializated variable was created to avoid calling init() more than once (because no other things are done during the FoBiSConfig class creation), but the first thing we are doing later is manually calling init() again.

May I suggest to redesign it?

@szaghi
Copy link
Owner

szaghi commented Jun 5, 2015

Dear Pepe, you can suggest all you want, I am learning Python.

You are touched the most cumbersome part of the code. FoBiS has been refactored at least two times just because this part was very unclear. As you guess, the __config__ is intended to avoid reinitialization, at least for its first purpose. For some reasons that now I do not remember the reset method was added and the first aim is missed.

As you suggest, I have to re-design this class and its usage.

Surely, your help is very appreciated.

Let's start...

  • which is the rigth convention for naming the configurator global variable? _config is good? I am not sure, but I think to have chosen the 4 underscores after linting the code with pep or pyflake;
  • I have to recheck the code, but if I rembember I was trying to:
    • reset the configurator any times I explicitely want to do;
    • initialize the class only once for simple script usage, but if used as a package (as it is done for the unittests) it must be reinitialized by the reset method.

How I can obtain such goals with a Pythonic programing? Maybe exploting the new method that I never used?

Thank you Pepe, see you later!

@szaghi
Copy link
Owner

szaghi commented Jun 5, 2015

A new Pepe snippet:

class FoBiSConfig(object):
  """
  Object handling FoBiS.py configuration
  """
  def __init__(self, fake_args=None):
    self.__cliparser = cliparser(fake_args)

  @property
  def cli(self):
    return self.__cliparser

  @cli.setter
  def cli(self, fake_args=None):
     self.__cliparser = cliparser(fake_args)

Meaning

A configurator should do nothing else that stores configurations... it is very cumbersome that FoBiS configurator actually contains the main working procedures.

Suggestion

Refactor the config in order to be a real config. Pay attention to the private variables (__var).

Exploit the suggested decorators for doing Pythonic usage of private variables.

Note

Using fake prefix can be unclear... maybe rename it.

@szaghi
Copy link
Owner

szaghi commented Nov 20, 2015

Done in current version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants