Paul Hankin wrote:
> On Nov 12, 3:25 pm, "[EMAIL PROTECTED]"
> <[EMAIL PROTECTED]> wrote:
>> Hi,
>>
>> I have recently been learning python, and coming from a java
>> background there are many new ways of doing things that I am only just
>> getting to grips with.
>>
>> I wondered if anyone could take a look at a few pieces of code I have
>> written to see if there are any places where I am still using java-
>> esque techniques, and letting me know the appropriate python way of
>> doing things.
>>
>> Here is a node class I wrote for use in graph traversal algorithms:
>>
>> #====
>>
>> class Node:
>> """
>> Node models a single piece of connected data.
>>
>> Author: Peter Braden
>> Last Modified : Nov. '07
>> """
>>
>> def __init__(self, connections = None, uid = None):
>> """
>> Args:
>> [connections - a list of (connected node, weight)
>> tuples. ]
>> [uid - an identifier for comparisons.]
>> """
>> self._connected = []
>> self._weights = []
>>
>> if connections:
>> for i in connections:
>> self.connected.append(i[0])
>> self.weights.append(i[1])
>>
>> if not uid:
>> self.id = id(self)
>> else:
>> self.id = uid
>>
>> def __eq__(self, other):
>> return self.id == other.id
>>
>> def getConnected(self):
>> return self._connected
>>
>> def getWeights(self):
>> return self._weights
>>
>> def getConnections(self):
>> connections = []
>> for i in range(len(connected)):
>>
>> connections.append((self._connected[i],self._weight[i]))
>> return connections
>>
>> connected = property(getConnected, None)
>> weights = property (getWeights, None)
>> connections = property(getConnections, None)
>>
>> def addConnection(self, node, weight = 0):
>> self.connected.append(node)
>> self.weights.append(weight)
>>
>> def removeConnection(self, node):
>> i = self._connected.index(node)
>> del self._connected[i]
>> del self._weights[i]
>
> There's no reason to make 'connected' and 'weights' properties: just
> use them directly.
>
> Make 'connections' default to [] rather than None, and replace the
> slightly clumsy initialisation with more direct code:
>
> self.connected = [i[0] for i in connections]
> self.weights = [i[1] for i in connections]
>
> getConnections can be much simpler...
>
> def getConnections(self):
> return zip(self.connected, self.weights)
>
> The 'uid' business makes me suspicious: what's it for? If you need it,
> you probably need to initialise with an explicit test for None rather
> than just 'if uid' which will be wrong if you use a uid of 0...
>
> self.id = uid if uid is not None else id(self)
>
> HTH
> --
> Paul Hankin
>
Be VERY careful with using [] as default arguments. Mutables are only
evaluated
once (not at every call). This question comes up about once a week on this
forum where people don't understand this.
I would recommend using (if you can):
def __init__(self, connected = None, weights=None, uid = None):
self.connected = connected or []
self.weights = weights or []
If you want to stay with single connections list do this:
def __init__(self, connections = None, uid = None):
if connections is not None:
self.connected=[c[0] for c in connections]
self.weights=[c(1) for c in connections]
else:
self.connected=[]
self.weights=[]
Others have addressed the lack of need for accessors.
-Larry
--
http://mail.python.org/mailman/listinfo/python-list
