Moving from java to python.

[PeterBraden1]

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,self._weight))
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
del self._weights

#===

Cheers,
Peter

 

[Jarek Zgoda]

(e-mail address removed) napisa³(a):

def getConnected(self):
return self._connected

No need to use accessors. Just plain attribute lookup is sufficient.

 

[Hrvoje Niksic]

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 camel-case naming
convention. If you must have multi-word method/variable/property
names, use lower-case 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,self._weight))

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.

 

[Paul Hankin]

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,self._weight))
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
del self._weights

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

 

[Tim Chase]

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,self._weight))
> 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 first-pass thoughts...

-tkc

 

[Larry Bates]

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,self._weight))
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
del self._weights

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

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