From: Brian M. <br...@gr...> - 2009-12-15 15:09:01
|
Richard, > Fascinating discussion. It is great to see these issues > being tackled. > > Just a thought. > > I would suggest adopting the 'Parameterise from Above' PfA > pattern. > > A quick read of some ACCU articles on the subject might be > diverting: > > The PfA Papers: From the Top > http://accu.org/index.php/journals/1411 > > The PfA Papers: The Clean Dozen > http://accu.org/index.php/journals/1420 > > ACCU :: The PfA Papers: Context Matters > http://accu.org/index.php/journals/1432 > > ACCU :: The PfA Papers: Deglobalisation > http://accu.org/index.php/journals/1470 Thanks for the excellent links. The PFA pattern couldn't be more appropriate for this discussion. I had begun to discover the pattern years ago before anyone told me the name of it. When a couple of years ago someone (maybe you) gave me the formal name and description, I felt like I had received a divine revelation. > The upshot of the PfA pattern, when we are talking Python, > is that > conditional import and implicit import magic is bad. PfA > would recommend > that knowledge of which mode of operation (gui or cli) > should be passed > in as an explicit parameter or more specifically in this > case the > actually error dialog to use should be passed as the > parameter. Even if > this means passing things around from one level of function > to another. > > The principle is that the module calling the error function > does not > need to know whether it is running in cli or gui mode. It > only needs to > have a handle to an error function. I couldn't agree more. Code like this should be outlawed: if use_gui: import gui.ErrorDialog as ErrorDialog elif use_cli: import cli.EfforDialog as ErrorDialog The code increases coupling and scales poorly. > In Python there are a number of ways to achieve PfA. Using > the example > of the ErrorDialog: > > 1) You could pass the ErrorDialog object into the function > that might > needed it: > > # file1 > class C: > def f(self, param1, > err_dlg=None): > if > err_dlg: > > err_dlg() > > # file2 > import file1 > import ErrorDialog > c=file1.C() > c.f(stuff, ErrorDialog) > > This can be very painful and you end up with loads of > things being > passed around as parameters that just clutter everything > up. > > 2) You can make the dependant object (ErrorDialog) a class > parameter. (I > won't insult your intelligence with the example code). This > is a bit > better but you still need to include it every time you > instantiate the > objects. > > 3) You could 'inject' the required object into the module > that needs it. > (code not checked, but I am sure you will get the idea). > > # file1 > def err_dlg(*args,**vargs): > log('Error dialog not > set!') > > def set_err_dlg(dlg): > global err_dlg > err_dlg = err_dlg > > class C: > def f(self, param1): > > err_dlg() > > # file2 > if GUI: > from > gui.widgets.dialogs import ErrorDialog as errormsg > else: > from cli.widgets > import errormsg > > import file1 > file1.set_err_dlg(errormsg) > c=file1.C() > c.f(stuff) > > I think that this is the best approach. You eliminate the > need for the > lower level modules (file1 in my example) to directly > import the > ErrorDialog. The knowledge of which version of the dialog > can be lifted > much higher in the abstraction tree. As a side effect you > also remove > the need for the modules containing the error dialogs to be > in the > import path when file1 is imported. This can be very useful > when writing > unit tests as it makes the test harness easier to write. > It also mean that the implementation of the error dialog > can be moved to > another module and only one import needs to be changed. I had previously put quite a bit of thought into this particular situation. I have recognized that there are multiple scenarios where the program may want to interact with the user: Errors, Warnings, Information, Progress Status. My idea is to combine all of these types of interaction into a single class. Here is how I think I would do it: === src/gen/ui.py === def class Ui: def inform_user(message): raise NotImplementedError def prompt_user(question): raise NotImplementedError def raise_error(error): raise NotImplementedError def notify_progress(progress): raise NotImplementedError === src/cli/cliui.py === def class CliUi(gen.Ui): def inform_user(message): print message + "\n" def prompt_user(question): print question + "\n" read answer return answer def raise_error(error): print error + "\n" def notify_progress(progress): print "Progress: %%d\n" % progress === src/gui/guiui.py === def class GuiUi(gen.Ui): def inform_user(message): gtk.Dialog(message, ... bla bla bla def prompt_user(question): answer = gtk.Dialog(question, ... bla bla bla return answer def raise_error(error): gtk.Dialog(error, ... bla bla bla def notify_progress(progress): gtk.progress_bar(progress, ... bla bla bla I haven't worked out the naming yet, obviously. Each interface would pass its own specific Ui instance into the plugin when launching it. The plugin would never know which user interface is actually in use. There are many other scenarios in Gramps where I think PAF should be used: const, config, DateDisplayer, NameDisplay, etc. It'll be a long road to get the code untangled. Any volunteers? ~Brian |