Re: parent-child object design question

[Steven D'Aprano]

On Fri, 02 Feb 2007 13:53:21 -0800, manstey wrote:

> Hi,
> 
> There was a mistake above, and then I'll explain what we're doing:
 insCacheClass = CacheClass(oref)
 insCacheProperty = CacheProperty(insOref,'Chapter')
> 
> should have been
 insCacheClass = CacheClass(oref)
 insCacheProperty = CacheProperty(insCacheClass ,'Chapter')
> 
> Now, to answer some questions.
> 1. Cache refers to Intersystems Cache database, an powerful oo dbase
> that holds our data.
> 2. It comes with a pythonbinding, but unfortunatley the API, written
> in C as a python extension, only provides old-style classes, which is
> why we wrap the oref (an in-memory instance of a Cache class/table)
> with CacheClass. This allows us to add attributes and methods to the
> CacheClass, the most important being CacheProperty, which is a class
> corresponding to the Cache property/field classes of the dbase.

You can add attributes and methods to old-style classes.


> 3. The pythonbind also has a strange behaviour, to our view. It gets
> and sets values of its properties (which are classes), 

Are you sure you're using the word "classes" correctly? That seems ...
strange. I'm wondering whether the properties are class _instances_.


> via the
> 'parent' oref, i.e. oref.set('Name','Peter'). But we want a pythonic
> way to interact with Cache, such as, insCacheClass.Name='Peter'. and
> insOref.Name.Set().

Okay, let me see that I understand what happens. Here's some pseudo-code
of what I think you are currently doing:


oref = get_a_reference_to_Cache_database()  # or whatever
# now do work on it using the python bindings.
oref.set("Name", "Peter")
oref.set("Something", "Else")


Here's a wrapper for the bindings, so they work more pythonically:

class Oref_Wrapper(object):
def __init__(self, oref):
self._oref = oref  # save the oref for later use
# Delegate attribute access to the oref
def __setattr__(self, name, value):
return self._oref.set(name, value)
def __getattr__(self, name):
return self._oref.get(name)  # or whatever the oref does
def __delattr__(self, name):
return self._oref.del(name)  # or whatever the oref does


This is how you use it:


oref = get_a_reference_to_Cache_database()  # or whatever
# re-bind the name to a wrapped version
oref = Oref_Wrapper(oref)
# now do work on it
oref.Name = "Peter"
oref.Something = "Else"


> The code above (in post 7) does this, but as I
> asked, by storing the parent instance (insCacheClass) inside each of
> its attributes (insCacheProperty1,2, etc).

Do you use insCacheClass and insCacheProperty for anything other than
making the bindings more Pythonic? Because for the life of me I can't work
out what they're supposed to do!


> 4. Another reason we want a new-style class wrapped around the oref,
> is that each oref has many methods on the server-side Cache database,
> but they are all called the same way, namely,
> oref.run_obj_method('%METHODNAME',[lisArgs]). We want to call these in
> python in the much better insCacheClass.METHODNAME(Args). E.g. we
> prefer insCacheClass.Save() to oref.run_obj_method('%Save',[None]).

Using my code above, oref.METHODNAME(args) would resolve to:

oref._oref.get("METHODNAME")(args)

which may not do what you want. 

If you know all the possible METHODNAMES, then that's easy enough to fix:
create methods at runtime. (If you don't know the methods, the problem
becomes too hard for me to work out at 3am, but will probably still be
solvable.)

Change the __init__ method above to something like this (untested):

import new

class Oref_Wrapper(object):
def __init__(self, oref):
self._oref = oref  # save the oref for later use
METHODNAMES = ["Save", "Clear", "Something"]
for name in METHODNAMES:
function = lambda self, *args: \
self._oref.run_obj_method("%"+name, args)
method = new.instancemethod(function, name)
self.__dict__[name] = method

then define the __getattr__ etc. methods as before, and (hopefully!) you
are done:

oref.Save()
oref.Clear(1, 2, 3, 7)



I hope this was of some help.



-- 
Steven.

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[manstey]

Hi,

There was a mistake above, and then I'll explain what we're doing:
>>> insCacheClass = CacheClass(oref)
>>> insCacheProperty = CacheProperty(insOref,'Chapter')

should have been
>>> insCacheClass = CacheClass(oref)
>>> insCacheProperty = CacheProperty(insCacheClass ,'Chapter')

Now, to answer some questions.
1. Cache refers to Intersystems Cache database, an powerful oo dbase
that holds our data.
2. It comes with a pythonbinding, but unfortunatley the API, written
in C as a python extension, only provides old-style classes, which is
why we wrap the oref (an in-memory instance of a Cache class/table)
with CacheClass. This allows us to add attributes and methods to the
CacheClass, the most important being CacheProperty, which is a class
corresponding to the Cache property/field classes of the dbase.
3. The pythonbind also has a strange behaviour, to our view. It gets
and sets values of its properties (which are classes), via the
'parent' oref, i.e. oref.set('Name','Peter'). But we want a pythonic
way to interact with Cache, such as, insCacheClass.Name='Peter'. and
insOref.Name.Set(). The code above (in post 7) does this, but as I
asked, by storing the parent instance (insCacheClass) inside each of
its attributes (insCacheProperty1,2, etc).
4. Another reason we want a new-style class wrapped around the oref,
is that each oref has many methods on the server-side Cache database,
but they are all called the same way, namely,
oref.run_obj_method('%METHODNAME',[lisArgs]). We want to call these in
python in the much better insCacheClass.METHODNAME(Args). E.g. we
prefer insCacheClass.Save() to oref.run_obj_method('%Save',[None]).
More importantly, the in-memory version of Cache lists and arrays are
Cache lists and arrays, but these are not Python lists and
dictionaries, but they can be easily converted into them. So having a
class for each dbase property allows us to have Cache on disk
(server), get the Cache list/array to python in memory, but still in
Cache dataformat (e.g. %ListOfDataTypes) and convert it to a python
list. Tweaking the set and get methods then lets us use python lists
and dics without ever worrying about cache dataformat.

I hope this clarifies what we are doing. We are not experienced
programmers, but we want to do it this way.

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[Steven D'Aprano]

On Wed, 31 Jan 2007 15:09:29 -0800, manstey wrote:

> Thanks for your input. Here is my next version, which works very well,
> but for one problem I explain below:
> 
> class CacheProperty(object):
> def __init__(self, insCacheClass, name):
> self.Name = name
> self._bind_to_parent(insCacheClass)
> self.__parent = insCacheClass
> self.__pyValue = self.__parent.get(name)
> 
> def _bind_to_parent(self, parent):
> setattr(parent, self.Name, self)
> 
> def get(self):
> return self.__pyValue
> 
> def set(self, val):
> self.__parent.set(self.Name, val)
> self.__pyValue = val # Set in wrapper's copy
> 
> Value = property(get, set)
> 
> def __repr__(self):
> return str(self.Value)
> def __str__(self):
> return str(self.Value)
> 
> 
> class CacheClass(object):
> def __init__(self, obj):
> self.__data = obj
> 
> def getOref(self):
> return self.__data

That's pointless. Just get rid of the double-underscores and let the
caller refer to self.data directly.

> def __repr__(self):
> return 'self.__data'

What's the purpose of this? Why do completely different instances of
CacheClass have exactly the same repr?


> def __str__(self):
> return str(self.__data)

Pointless, because __getattr__ will have the same effect: if __str__
doesn't exist, __getattr__ will call getattr(self.__data, "__str__") which
is equivalent to str(self.__data).


> def __getattr__(self, attr):
> return getattr(self.__data, attr)

Straight-forward delegation to self.__data. As near as I can see,
CacheClass(oref) essentially does nothing different to just oref. So why
wrap oref in another class? What's the purpose of this? Why not just refer
to oref in the first place?

What makes this a "cache" class? It doesn't seem to act anything like a
cache to me. It seems to be just a wrapper around oref that doesn't really
do anything.


> Our code works fine as follows (oref is in-memory version of Cache oo-
> dbase class, and is an old-style class that comes with set and get and
> run_obj_method methods):
 insCacheClass = CacheClass(oref)
 insCacheProperty = CacheProperty(insOref,'Chapter')

What is insOref and where does it come from? Is it different from oref?

Your classes are confusing, but I feel that all of the above is just a
big, complicated way of saying something like this:

insOref = something_or_other
insOref.some_name = insOref

and then jumping through all sorts of hoops writing this:

insOref.some_name.attribute

instead of just insOref.attribute


That's what it seems like to me. Have I missed something?


 print insOref.Chapter.Value
> 5
 print insOref.Chapter.Name
> 'Chapter'
 insOref.Chapter.Value=67
 print insOref.Chapter
> 67
> 
> However, the problem is now that I can also write:
 insOref.Chapter=67
> but we want to disallow this, as insOref.Chapter must remain =
> insProperty

Why?



> We add various other instances of CacheProperty to the insOref as
> well, btw.

This whole design seems incredibly byzantine to me. It would help if you
take a step back from asking technical questions "how do I do this?" and
instead tell us what you're trying to accomplish.

Why do you wrap the instances in the first place? What benefit do you get?

Here is something that you should do. Write your documentation for the
classes CacheProperty and CacheClass. The documentation should explain
what they are for, why you would use them, and how you use them. If you
can't explain what they are for and why you would use them, chances are
very very good that they don't do anything useful and you shouldn't use
them at all.




-- 
Steven D'Aprano 

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[Steven D'Aprano]

On Wed, 31 Jan 2007 20:15:44 +1100, Ben Finney wrote:

> "Steven D'Aprano" <[EMAIL PROTECTED]> writes:
> 
>> > def _accumulate_properties(self, properties):
>> > self.properties = []
>>
>> Probably better to put that in the __init__ method, otherwise if
>> somebody runs instance._accumulate_properties(...) again, it will
>> have the side-effect of throwing away whatever was already in
>> instance.properties.
> 
> That's the point of naming it with a leading underscore. You've
> already explained that this is convention for "Private, don't use".

"Somebody" could easily be the class designer. This is a method that, as
written, will break the instance if it is run twice. That's not good.

 
>> Some people might argue that if someone runs a private method like
>> _accumulate_properties, they deserve whatever bad things happen to
>> them.  But I say, well, sure, but why *design* your method to break
>> things when called twice?
> 
> Exactly the same could be said for calling the __init__ method twice.

__init__ isn't a "private" method, it is a public method. A very special
public method, designed to initialise the instance, that is to set the
instance to a brand new pristine state.

Calling instance.__init__ *should* initialise the instance and throw away
whatever data was already there. (Why you would want to do this is another
question.) What else would you expect it to do? If you called __init__ and
it *didn't* initialise the instance, that would surely be a bug.

But _accumulate_properties as written is sort of half-man-half-duck sort
of thing: it would be useful for accumulating properties to the instance,
except that when you do so it throws away whatever properties you have
previously accumulated. Or rather, it doesn't even do that -- it simply
throws away the list of properties, but leaves the actual properties
behind. That can't possibly be good design! An inconsistent internal
state like that is a bug waiting to happen.


>> Who knows, maybe you'll decide you want to call it twice yourself.
> 
> Right. In which case, it's good that it's already in a separate,
> clearly-named, single-purpose method.

It isn't either clearly-named or single-purpose.

The name is misleading, for two reasons. First, "properties" has a
technical meaning in Python, and this method doesn't have anything to do
with that meaning. Secondly, even if it did, it doesn't accumulate them,
it resets the list to a new state, throwing away whatever was there before. 

Nor is it single-purpose: while it *sets* a list of attribute names to a
new list of values, it *adds* those new attributes to the instance __dict__.

There are some methods that, in principle, should be run once and once
only. There's nothing inherently about accumulating attributes/properties
to an instance can only be done once. But the way the method is written,
it will break things if you try to accumulate attributes twice -- not
because of any inherent badness in doing so, but because the method breaks
the consistency between self.properties (a list) and the actual attributes
accumulated.

> Make the code easy to understand, not idiot-proof.

I'm not arguing that the entire method should be moved into __init__.
*Initialising* self.properties list to [] is not logically part of
*accumulating* properties, and should be moved out of that method. That
tiny change will fix all of the problems I describe except for the
misleading name.



-- 
Steven D'Aprano 

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[Ben Finney]

"manstey" <[EMAIL PROTECTED]> writes:

> However, the problem is now that I can also write:
> >>> insOref.Chapter=67
> but we want to disallow this, as insOref.Chapter must remain =
> insProperty

Then don't do that.

Python allows any name to be reassigned to any value, with the
attitude of "we're all consenting adults here". It's better to
document[0] the correct behaviour of
your code, rather than trying to prevent stupid mistakes.


[0]: and unit tests, that explicitly check the behaviour of the code,
and get run all the time during development of the code, are the best
way of documenting behaviour unambiguously.

-- 
 \ "Buy not what you want, but what you need; what you do not need |
  `\   is expensive at a penny."  -- Cato, 234-149 BC, Relique |
_o__)  |
Ben Finney

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[manstey]

Thanks for your input. Here is my next version, which works very well,
but for one problem I explain below:

class CacheProperty(object):
def __init__(self, insCacheClass, name):
self.Name = name
self._bind_to_parent(insCacheClass)
self.__parent = insCacheClass
self.__pyValue = self.__parent.get(name)

def _bind_to_parent(self, parent):
setattr(parent, self.Name, self)

def get(self):
return self.__pyValue

def set(self, val):
self.__parent.set(self.Name, val)
self.__pyValue = val # Set in wrapper's copy

Value = property(get, set)

def __repr__(self):
return str(self.Value)
def __str__(self):
return str(self.Value)


class CacheClass(object):
def __init__(self, obj):
self.__data = obj

def getOref(self):
return self.__data
def __repr__(self):
return 'self.__data'
def __str__(self):
return str(self.__data)
def __getattr__(self, attr):
return getattr(self.__data, attr)

Our code works fine as follows (oref is in-memory version of Cache oo-
dbase class, and is an old-style class that comes with set and get and
run_obj_method methods):
>>> insCacheClass = CacheClass(oref)
>>> insCacheProperty = CacheProperty(insOref,'Chapter')
>>> print insOref.Chapter.Value
5
>>> print insOref.Chapter.Name
'Chapter'
>>> insOref.Chapter.Value=67
>>> print insOref.Chapter
67

However, the problem is now that I can also write:
>>> insOref.Chapter=67
but we want to disallow this, as insOref.Chapter must remain =
insProperty

We add various other instances of CacheProperty to the insOref as
well, btw.

So any idea how to prohibit this, and can the class code above be
improved? Does it matter that each instance of CacheProperty contains
self.__parent? Does it actually "contain" the parent (I realise this
is not 'parent' in usual python lingo - what would be a better term?),
or is self.__parent simply point to it somehow? I don't understand
this part of python at all!

Thanks, you are being a great help in our development.

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[Ben Finney]

"Steven D'Aprano" <[EMAIL PROTECTED]> writes:

> > def _accumulate_properties(self, properties):
> > self.properties = []
>
> Probably better to put that in the __init__ method, otherwise if
> somebody runs instance._accumulate_properties(...) again, it will
> have the side-effect of throwing away whatever was already in
> instance.properties.

That's the point of naming it with a leading underscore. You've
already explained that this is convention for "Private, don't use".

> Some people might argue that if someone runs a private method like
> _accumulate_properties, they deserve whatever bad things happen to
> them.  But I say, well, sure, but why *design* your method to break
> things when called twice?

Exactly the same could be said for calling the __init__ method twice.

> Who knows, maybe you'll decide you want to call it twice yourself.

Right. In which case, it's good that it's already in a separate,
clearly-named, single-purpose method.

Make the code easy to understand, not idiot-proof.

-- 
 \  "One time a cop pulled me over for running a stop sign. He |
  `\said, 'Didn't you see the stop sign?' I said, 'Yeah, but I |
_o__) don't believe everything I read.'"  -- Steven Wright |
Ben Finney

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[Steven D'Aprano]

On Tue, 30 Jan 2007 21:15:53 -0800, manstey wrote:

> Hi Ben,
> 
> Could I also do something like the following? What does it mean to
> store the parent class as a private variable in the child class?

What it means is that references to "self.__data" (note the TWO leading
underscores) in your code below are mangled by Python to be
"self._CacheProperty__data__" instead.

This gives you a weak form of "private" variable, good for annoying people
and yourself, possibly okay to help prevent many accidental name clashes,
but not much else.

On the other hand, references to "self._parent" (note only ONE leading
underscore) is a convention. It is like putting a comment "Private, don't
touch" in your code. It isn't enforced by Python. That's usually a good
thing.


> class CacheProperty(object):
> def __init__(self, obj, parent, properties=None):
> self.__data = obj
> self._parent = parent
> if properties is None:
> properties = {}
> self._accumulate_properties(properties)
> 
> 
> def _accumulate_properties(self, properties):
> self.properties = []

Probably better to put that in the __init__ method, otherwise if somebody
runs instance._accumulate_properties(...) again, it will have the
side-effect of throwing away whatever was already in instance.properties.

Some people might argue that if someone runs a private method like
_accumulate_properties, they deserve whatever bad things happen to them.
But I say, well, sure, but why *design* your method to break things when
called twice? Who knows, maybe you'll decide you want to call it twice
yourself.


> for key, val in properties.iteritems():
> setattr(self, key, val)
> self.properties.append(key)

If I am reading this correctly, self.properties is almost the same as
self.__dict__.keys() only missing some values.


> def __getattr__(self, name):
> return getattr(self.__data, name)

Okay, let's see how this works.

child = CacheProperty([1,2,3], PARENT, {"food": "eggs", "colour": "green"})

# child.__data is the list [1,2,3]
# child._parent is PARENT (whatever that is)
# child.properties is the list ['food', 'colour']

Now, if I say:

child.__len__()
=> returns 3

child.index(2)
=> returns 1

That's just straight-forward delegation to the "obj" argument stored in
data. But when I say:

child.set('foo')

expecting to have the following method called:

> def set(self, property):
> return self._parent.set(property)

oops! It gets delegated to obj instead of the parent.

> the set function allows us to call the parent set function within the
> child, which is what we need to do.

Why not do this?

instance._parent.set('foo')

No mess, no fuss, bug-free, and self-documenting.


-- 
Steven D'Aprano 

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[Ben Finney]

"manstey" <[EMAIL PROTECTED]> writes:

> Could I also do something like the following?

I can't immediately see a problem with the code you posted. Does it do
what you want it to do?

> What does it mean to store the parent class as a private variable in
> the child class?

I don't understand this question. It's up to you what it means; I
leave it to the people who know the purpose of the program to decide
whether "parent" is even an appropriate term.

-- 
 \"I took a course in speed waiting. Now I can wait an hour in |
  `\  only ten minutes."  -- Steven Wright |
_o__)  |
Ben Finney

-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[manstey]

Hi Ben,

Could I also do something like the following? What does it mean to
store the parent class as a private variable in the child class?

class CacheProperty(object):
def __init__(self, obj, parent, properties=None):
self.__data = obj
self._parent = parent
if properties is None:
properties = {}
self._accumulate_properties(properties)


def _accumulate_properties(self, properties):
self.properties = []
for key, val in properties.iteritems():
setattr(self, key, val)
self.properties.append(key)


def __getattr__(self, name):
return getattr(self.__data, name)

def set(self, property):
return self._parent.set(property)

the set function allows us to call the parent set function within the
child, which is what we need to do.



-- 
http://mail.python.org/mailman/listinfo/python-list

Re: parent-child object design question

[Ben Finney]

"manstey" <[EMAIL PROTECTED]> writes:

> I have two classes. The first one wraps around an old-style class
> called oref
>
> Class CacheClass(object):
> def __init__(self, obj):
> self.__data = obj
> def __getattr__(self, attr):
> return getattr(self.__data, attr)

I presume the 'obj' argument to this class's __init__ method contains
the 'oref' instance.

It's a little confusing to call the argument to __getattr__ "attr",
since it's actually the *name* of the attribute to be accessed, not
the attribute itself.

> The second class is much the same, also wrapping, but has some
> attributes.
>
> class CacheProperty(object):
> def __init__(self, obj, dicProperties={}):

Setting a container class as a default argument is prone to error; the
default argument gets created once, when the 'def' statement is
executed. Better to default to None, and trigger on that inside the
function.

> self.__data = obj
> lisProperties=[]
> for key, val in dicProperties.iteritems():
> setattr(self, key, val)
> lisProperties.append(key)
> self.Properties = lisProperties

The name of this class doesn't really tell me what an instance of the
class *is*. Is it a "cache property", as the name seems to indicate?
If so, why does a "cache property" instance itself contain a list of
properties?

> def __getattr__(self, attr):
> return getattr(self.__data, attr)
>
> Here is my code:
>
> >>> MyClass = CacheClass(oref)

This is a confusing example; MyClass implies that the object is a
class, but this is now an instance of CacheClass, not a class.

Also, it's conventional in Python to name classes in TitleCase, but to
name instances (and functions) starting with lowercase.

> >>> MyProperty = CacheProperty(oref2,{'Name':'Surname', 'Value':'Peter'})
> >>> setattr(MyClass,MyProperty.Name,MyProperty)

This might be a good approach if you didn't know you were going to
associate the object MyClass with the object MyProperty at the
creation of MyProperty. However:

> Now, the problem is that I want a method MyClass.MyProperty.Save()
> to save the value of MyClass.MyProperty to the database backend on
> disk, but the code for this is part of the wrapped oref code, and
> thus is invoked only by
> MyClass.set(MyProperty.Name,MyProperty.Value).

This implies that there's a definite "each CacheProperty instance is
associated with exactly one CacheClass instance" invariant.

If that's true, the best thing to do is to pass MyClass to the
constructor for the CacheProperty class, and have each instance set
the relationship in the __init__ method.

I don't know what a descriptive term for the relationship between the
CacheClass instance and the CacheProperty instance is, so I'm going to
use "parent"; you should choose something more descriptive.

class CacheProperty(object):
def __init__(self, obj, parent, properties=None):
self.__data = obj
self._bind_to_parent(parent)
if properties is None:
properties = {}
self._accumulate_properties(properties)

def _bind_to_parent(self, parent):
setattr(parent, self.Name, self)

def _accumulate_properties(self, properties):
self.properties = []
for key, val in properties.iteritems():
setattr(self, key, val)
self.properties.append(key)

def __getattr__(self, name):
return getattr(self.__data, name)

-- 
 \ "Marriage is a wonderful institution, but who would want to |
  `\ live in an institution."  -- Henry L. Mencken |
_o__)  |
Ben Finney

-- 
http://mail.python.org/mailman/listinfo/python-list

parent-child object design question

[manstey]

Hi,

I am having trouble designing my classes.

I have two classes. The first one wraps around an old-style class 
called oref


Class CacheClass(object):
def __init__(self, obj):
self.__data = obj
def __getattr__(self, attr):
return getattr(self.__data, attr)

The second class is much the same, also wrapping, but has some 
attributes.

class CacheProperty(object):
def __init__(self, obj, dicProperties={}):
self.__data = obj
lisProperties=[]
for key, val in dicProperties.iteritems():
setattr(self, key, val)
lisProperties.append(key)
self.Properties = lisProperties
def __getattr__(self, attr):
return getattr(self.__data, attr)

Here is my code:

>>> MyClass = CacheClass(oref)
>>> MyProperty = CacheProperty(oref2,{'Name':'Surname', 'Value':'Peter'})
>>> setattr(MyClass,MyProperty.Name,MyProperty)

Now, the problem is that I want a method MyClass.MyProperty.Save() to 
save the value of MyClass.MyProperty to the database backend on disk, 
but the code for this is part of the wrapped oref code, and thus is 
invoked only by MyClass.set(MyProperty.Name,MyProperty.Value).

How can I access this method inside the MyProperty class?

I hope this is clear!

regard,s
matthew

-- 
http://mail.python.org/mailman/listinfo/python-list