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:

>>> id = idgen('01',4)
>>> id 01_20060424_151903_1 >>> id 01_20060424_151905_2 >>> id 01_20060424_151905_3 >>> id 01_20060424_151906_4 >>> id 01_20060424_151907_1 >>>
>>> id = idgen(04,100)
>>> id

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 -----

--
mph

[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:

id = IDGenerator(...)
id() 01_20060424_151903_1 id()

01_20060424_151905_2

[keirr]

Martin P. Hellwig wrote:

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]

Bruno Desthuilliers wrote:

Martin P. Hellwig a écrit :

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:

>>> id = IDGenerator(...)
>>> id() 01_20060424_151903_1 >>> id()

01_20060424_151905_2

because that shadows a builtin?

sorry, could not resist :-)

Cheers,

Carl Friedrch Bolz

[Carl Friedrich Bolz]

Bruno Desthuilliers wrote:

Martin P. Hellwig a écrit :

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:

>>> id = IDGenerator(...)
>>> id() 01_20060424_151903_1 >>> id()

01_20060424_151905_2

because that shadows a builtin?

sorry, could not resist :-)

Cheers,

Carl Friedrch Bolz

[Martin P. Hellwig]

Bruno Desthuilliers wrote:
<cut>

Why not just use the call operator instead ? ie:

>>> id = IDGenerator(...)
>>> id() 01_20060424_151903_1 >>> id() 01_20060424_151905_2

Because of:

id = IDGenerator("01",99)
id() 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!

--
mph

[Martin P. Hellwig]

keirr wrote:

Martin P. Hellwig wrote:

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.

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.

--
mph

[Heiko Wundram]

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

Because of:

> id = IDGenerator("01",99)
> id()

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!

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]

Carl Friedrich Bolz wrote:

Bruno Desthuilliers wrote:

Martin P. Hellwig a écrit :

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:

>>> id = IDGenerator(...)
>>> id()

01_20060424_151903_1

>>> id()

01_20060424_151905_2

because that shadows a builtin?

oops :(
sorry, could not resist :-)

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

--
bruno desthuilliers
python -c "print '@'.join(['.'.join([w[::-1] for w in p.split('.')]) for
p in 'o****@xiludom.gro'.split('@')])"

[bruno at modulix]

Martin P. Hellwig wrote:

Bruno Desthuilliers wrote:
<cut>

Why not just use the call operator instead ? ie:

>>> id = IDGenerator(...)
>>> id()

01_20060424_151903_1

>>> id()

01_20060424_151905_2

Because of:

> id = IDGenerator("01",99)
> id()

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

>

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__.

--
bruno desthuilliers
python -c "print '@'.join(['.'.join([w[::-1] for w in p.split('.')]) for
p in 'o****@xiludom.gro'.split('@')])"

[Martin P. Hellwig]

bruno at modulix wrote:

Martin P. Hellwig wrote:

Bruno Desthuilliers wrote:
<cut>

Why not just use the call operator instead ? ie:

>>> id = IDGenerator(...)
>>> id()
01_20060424_151903_1
>>> id()
01_20060424_151905_2

Because of:

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

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!

--
mph

[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:

>>> id = IDGenerator('01',2)
>>> id() '01_20060504_112304_1' >>> id() '01_20060504_112306_2' >>> id() '01_20060504_112307_1' >>>
>>> id = IDGenerator(0005,99) # Note that an int will be cast to a string!
>>> id() '5_20060504_112324_01' >>> id() '5_20060504_112327_02' >>> id <class '__main__.IDGenerator'> previous ID is 5_20060504_112324_01 and
current ID is 5_20060504_112327_02 >>>

"""

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 -----