P: n/a

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]
#===
Cheers,
Peter  
Share this Question
P: n/a
 Pe**********@googlemail.com napisał(a):
def getConnected(self):
return self._connected
No need to use accessors. Just plain attribute lookup is sufficient.

Jarek Zgoda
Skype: jzgoda  GTalk: zg***@jabber.aster.pl  voice: +48228430101
"We read Knuth so you don't have to." (Tim Peters)  
P: n/a

On Nov 12, 3:25 pm, "PeterBrad...@googlemail.com"
<PeterBrad...@googlemail.comwrote:
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  
P: n/a

"Pe**********@googlemail.com" <Pe**********@googlemail.comwrites:
def __init__(self, connections = None, uid = None):
You can use connections=(), so you don't need the "if" below. In
fact, you can further write:
for node, weight in connections:
self._connected.append(node)
self._weight.append(weight)
def getConnected(self):
return self._connected
I'd drop the getters and simply use self.connected, self.weights, etc.
It's not such a big deal in Python if someone can view the innards of
your objects; they can do so even if their name starts with _.
Also note that Python doesn't really use the camelcase naming
convention. If you must have multiword method/variable/property
names, use lowercase with underscores for separation. For example:
def getConnections(self):
Why not simply def connections(self) or, if you must, def
get_connections(self).
connections = []
for i in range(len(connected)):
connections.append((self._connected[i],self._weight[i]))
Use xrange(len(...)) when iterating over the sequence; range
unnecessarily allocates the whole list. Some will probably recommend
enumerate() or even itertools.izip(), but those are overkill for this
usage.  
P: n/a

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])
I'd likely write this something like
if connections:
self.connected, self.weights = zip(*connections)
You also shift here between "_connected" and "connected" (same
with weights). This might bite you.
if not uid:
self.id = id(self)
else:
self.id = uid
A common way of writing this would be
self.id = uid or id(self)
However, the need for anything other than the id() is suspect.
Imaginable, but suspect.
def getConnected(self):
return self._connected
def getWeights(self):
return self._weights
connected = property(getConnected, None)
weights = property (getWeights, None)
No need for properties at this time...they can be added
transparently later, if you need to do something programatic on
access.
def getConnections(self):
connections = []
for i in range(len(connected)):
connections.append((self._connected[i],self._weight[i]))
return connections
connections = property(getConnections, None)
And in an inverse of the __init__ method, I'd use something like
def getConnections(self):
return zip(self.connected, self.weights)
Just my firstpass thoughts...
tkc  
P: n/a

Paul Hankin wrote:
On Nov 12, 3:25 pm, "PeterBrad...@googlemail.com"
<PeterBrad...@googlemail.comwrote:
>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   This discussion thread is closed Replies have been disabled for this discussion.   Question stats  viewed: 1149
 replies: 5
 date asked: Nov 12 '07
