This coding style bad practise?

[Martin P. Hellwig]

Hi all,

I created a class which creates a relative unique id string, now my
program just works fine and as expected but somehow I get the feeling
that I misused the __repr__ since I guess people expect to 'execute' a
function in an instance instead of using it's representation string of
the instance itself, could you elaborate whether you find this bad
practice and if yes what would have been a better way to do it?

TIA

----- script -----

import string
import time

class IDGenerator(object):
"""(serverID,subversion_length)
Create an ID from the server name, datetimestamp and version number.
Calling the instance returns the ID using the __repr__ class function

Example usage:
4_20060424_152043_001

"""

def __init__(self,serverID,subversion_length):
self.ID = str(serverID)
self.length = int(subversion_length)
fill_length = len(str(self.length))

def fill(number):
return(string.zfill(number,fill_length))
self.fill = fill


def __repr__(self):
# If the subversion length has been reached or the generator has not
# been defined, (re)define it, otherwise return the next value of the
# subversion.
try:
return_value = self.range_gen.next()

except:
self.range_gen = ( number for number in range(1,self.length+1) )
return_value = self.range_gen.next()

# Create the version stamp.
return_value = self.ID +\
time.strftime("_%Y%m%d_%H%M%S_",time.gmtime())+\
self.fill(return_value)

# And return it.
return(return_value)

----- script -----

 

[keirr]

Hi all,

I created a class which creates a relative unique id string, now my
program just works fine and as expected but somehow I get the feeling
that I misused the __repr__ since I guess people expect to 'execute' a
function in an instance instead of using it's representation string of
the instance itself, could you elaborate whether you find this bad
practice and if yes what would have been a better way to do it?

TIA

Rather than comment on the style of using a class, I'll just suggest an
alternative.
You can use a generator function, which yields the next id when called;
at least that's
how I'd implement an IDgenerator.

Cheers,

Keir.

 

[Carl Friedrich Bolz]

Martin P. Hellwig a écrit :

Why not just use the call operator instead ? ie:

01_20060424_151905_2

because that shadows a builtin?

sorry, could not resist

Cheers,

Carl Friedrch Bolz

 

[Carl Friedrich Bolz]

Martin P. Hellwig a écrit :

Why not just use the call operator instead ? ie:

01_20060424_151905_2

because that shadows a builtin?

sorry, could not resist

Cheers,

Carl Friedrch Bolz

 

[Martin P. Hellwig]

Bruno Desthuilliers wrote:

Why not just use the call operator instead ? ie:

01_20060424_151905_2

Because of:

Traceback (most recent call last):
File "<pyshell#1>", line 1, in ?
id()
TypeError: 'IDGenerator' object is not callable

But i do appreciate your comment, thanks!

 

[Martin P. Hellwig]

Rather than comment on the style of using a class, I'll just suggest an
alternative.
You can use a generator function, which yields the next id when called;
at least that's
how I'd implement an IDgenerator.

Cheers,

Keir.

Thanks for your comment, I do use a generator in my class because I
wanted to wrap my subversion when its on its end I had a choice of
looping in with an if check or using a generator with try except, this
time I choose a generator, though mostly I directly use a generator for
these type of functions.

 

[Bruno Desthuilliers]

Martin P. Hellwig a écrit :

Hi all,

I created a class which creates a relative unique id string, now my
program just works fine and as expected but somehow I get the feeling
that I misused the __repr__ since I guess people expect to 'execute' a
function in an instance instead of using it's representation string of
the instance itself, could you elaborate whether you find this bad
practice and if yes what would have been a better way to do it?

Why not just use the call operator instead ? ie:
01_20060424_151905_2

 

[Heiko Wundram]

Am Donnerstag 04 Mai 2006 01:04 schrieb Martin P. Hellwig:

Because of:

But i do appreciate your comment, thanks!

You need to define a __call__(self)-method on your class so that instances are
callable... Basically, what Bruno was saying, is that you rename your
__repr__(self) to __call__(self), and see what happens.

--- Heiko.

 

[bruno at modulix]

because that shadows a builtin?

oops

sorry, could not resist

<op>
idgen = IDGenerator(...)
idgen()

 

[bruno at modulix]

Bruno Desthuilliers wrote:


Because of:

Of course - you have to overload the call operator for this to work.
Just rename IDGenerator.__repr__ to IDGenerator.__call__, and I garantee
this will work - and will be *much* more cleaner than abusing __repr__.

 

[Martin P. Hellwig]

Of course - you have to overload the call operator for this to work.
Just rename IDGenerator.__repr__ to IDGenerator.__call__, and I garantee
this will work - and will be *much* more cleaner than abusing __repr__.

Thanks! That was the thing I was looking for!

 

[Martin P. Hellwig]

<cut>

Thanks for the input folks!
I adapted my script to the given suggestions and it's now far more
'logical', for reference I added it below.

--
mph

----- script -----

import string
import time

class IDGenerator(object):
"""(leading_id, subversion_length, tz) # tz = 'local' or 'gm' (default)
Create an ID from a given string, a current datetimestamp and version
number which wraps around at given subversion_length.

Example usage:

"""

def __init__(self,leading_id, subversion_length, timezone='gm'):
self.id = str(leading_id)
self.length = int(subversion_length)
fill_length = len(str(self.length))
self.current = None
self.previous = None

def fill(number):
return(string.zfill(number,fill_length))
self.fill = fill

if timezone == 'local':
self.timeset = time.localtime
else:
self.timeset = time.gmtime


def __call__(self):
# If the subversion length has been reached or the generator has not
# been defined, (re)define it, otherwise return the next value of the
# subversion.
try:
return_value = self.range_gen.next()

except:
self.range_gen = ( number for number in range(1,self.length+1) )
return_value = self.range_gen.next()

# Create the version stamp.
return_value = self.id +\
time.strftime("_%Y%m%d_%H%M%S_",self.timeset())+\
self.fill(return_value)

# Copy the current ID to the previous and assign a new one to current.
self.previous = self.current
self.current = return_value

# And return it.
return(self.current)

def __repr__(self):
return(str(self.__class__) +
' previous ID is ' +
str(self.previous) +
' and current ID is ' +
str(self.current))

----- script -----