Re: oo design question - how to avoid a factory

On Sat, 18 Dec 2004 17:49:31 +0100, Olivier Guilyardi <ml@xung.org> wrote: > Hi, > > Justin Patrin wrote: > > On Fri, 17 Dec 2004 12:39:36 -0500, Andrew Nagy > > <andrew.nagy@villanova.edu> wrote: > > > >>Could using a factory instead of a constructor be confusing to the > >>non-savvy user? > >> > > Andrew, a factory isn't confusing by itself. But it is if a new version > of Structures_DataGrid requires to use a factory, thus breaking bc. > > As I stated when I sent this diff to you : "So... It's good we're still > in alpha stage because I believe we can't do without a factory." > > But, still, this "alpha" status is a bit theoritical, because AFAICS, > according to pear-general and pear-dev, Structures_DataGrid does have > many users. And, for sure, the next release is going to be confusing > to all of these, if the factory is required : > typing "pear upgrade Structures_DataGrid" will break their software > > > I don't see a factory as being too confusing, as long as you *always* > > use the factory in examples and make it clear in the docs that you > > should (must) use the factory. > > > > If you want to avoid the factory you can always make the user call > > setRenderer() after a 'new' call. Or call it internally whenever you > > need to use the renderer instead of instantiating it at "new" time. > > That's right, I thought about this before coding that factory, but there > are many methods in Structures_DataGrid that would need to check if > the object is already fully instantiated (bind(), addRecord(), > addColumn(), etc...). That would be heavy and dirty. > > Now, there may be another solution, to keep bc : In the constructor, > issue a warning like "Please use the factory...", and then act as > a decorator to the real datagrid. This is very possible because the > Structures_DataGrid class is currently an almost empty wrapper. > > Check it out : > > // Can't extend Structures_DataGrid_Renderer anymore : the decorating > // methods would create a conflict with the decorated ones. > > class Structures_DataGrid > { > var $_decorated; > > function Structures_DataGrid($limit = null, $page = 1, > $renderer = DATAGRID_RENDER_TABLE) > { > PEAR::raiseError("Please use the factory"); > $this->_decorated =& Structures_DataGrid::factory($limit, $page, $renderer); > } > > function &factory($limit = null, $page=1,$renderer = DATAGRID_RENDER_TABLE) > { > // Notice that I instantiate the renderer class directly : > > $dg = new Structures_DataGrid_Renderer($limit, $page); > $dg->setRenderer($renderer); > return $dg; > } > > // Now decorating public methods. These wrappers won't get called by people > // who used the factory. > > function bind($rs) > { > return $this->_decorated->bind($rs); > } > > function addColumn($column) > { > return $this->_decorated->addColumn($column); > } > > etc.... > } > > One may say that it's heavy, but it's not for the one who directly use > the factory. All of this workaround will only create a bit of overhead for > people who didn't RTFM recently, but will keep bc. And it could be wiped > out in some future major release. > > What do you all think about this idea ? Is it an acceptable trick ? > Well, it would include lots of extra code by default and be useless to those who read the docs. If Structures_DataGrid has yet to go stable BC breaks are ok. You should put a large statement in the changelog which tells the user that there are BC breaks. If their apps fail, they can ask on the list and be redirected to the docs. I say don't put in the large ugly hack. It will just make support harder. -- Justin Patrin