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 -----[color=blue]
> import string
> import time
>
> class IDGenerator(obj ect):
> """(serverID,su bversion_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:[color=green][color=darkred]
> >>> id = idgen('01',4)
> >>> id[/color][/color]
> 01_20060424_151 903_1[color=green][color=darkred]
> >>> id[/color][/color]
> 01_20060424_151 905_2[color=green][color=darkred]
> >>> id[/color][/color]
> 01_20060424_151 905_3[color=green][color=darkred]
> >>> id[/color][/color]
> 01_20060424_151 906_4[color=green][color=darkred]
> >>> id[/color][/color]
> 01_20060424_151 907_1[color=green][color=darkred]
> >>>
> >>> id = idgen(04,100)
> >>> id[/color][/color]
> 4_20060424_1520 43_001
>
> """
>
> def __init__(self,s erverID,subvers ion_length):
> self.ID = str(serverID)
> self.length = int(subversion_ length)
> fill_length = len(str(self.le ngth))
>
> def fill(number):
> return(string.z fill(number,fil l_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.le ngth+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(retur n_value)
>
> # And return it.
> return(return_v alue)[/color]
----- script -----

--
mph

[Bruno Desthuilliers]

Re: This coding style bad practise?

Martin P. Hellwig a écrit :[color=blue]
> 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?[/color]

Why not just use the call operator instead ? ie:
[color=blue][color=green][color=darkred]
>>> id = IDGenerator(... )
>>> id()[/color][/color][/color]
01_20060424_151 903_1[color=blue][color=green][color=darkred]
>>> id()[/color][/color][/color]
01_20060424_151 905_2

[keirr]

Re: This coding style bad practise?


Martin P. Hellwig wrote:[color=blue]
> 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
>[/color]

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]

Re: This coding style bad practise?

Bruno Desthuilliers wrote:[color=blue]
> Martin P. Hellwig a écrit :[color=green]
>>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?[/color]
>
> Why not just use the call operator instead ? ie:
>[color=green][color=darkred]
> >>> id = IDGenerator(... )
> >>> id()[/color][/color]
> 01_20060424_151 903_1[color=green][color=darkred]
> >>> id()[/color][/color]
> 01_20060424_151 905_2[/color]

because that shadows a builtin?

sorry, could not resist :-)

Cheers,

Carl Friedrch Bolz

[Carl Friedrich Bolz]

Re: This coding style bad practise?

Bruno Desthuilliers wrote:[color=blue]
> Martin P. Hellwig a écrit :[color=green]
>>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?[/color]
>
> Why not just use the call operator instead ? ie:
>[color=green][color=darkred]
> >>> id = IDGenerator(... )
> >>> id()[/color][/color]
> 01_20060424_151 903_1[color=green][color=darkred]
> >>> id()[/color][/color]
> 01_20060424_151 905_2[/color]

because that shadows a builtin?

sorry, could not resist :-)

Cheers,

Carl Friedrch Bolz

[Martin P. Hellwig]

Re: This coding style bad practise?

Bruno Desthuilliers wrote:
<cut>[color=blue]
>
> Why not just use the call operator instead ? ie:
>[color=green][color=darkred]
> >>> id = IDGenerator(... )
> >>> id()[/color][/color]
> 01_20060424_151 903_1[color=green][color=darkred]
> >>> id()[/color][/color]
> 01_20060424_151 905_2
>[/color]

Because of:[color=blue][color=green][color=darkred]
>>>> id = IDGenerator("01 ",99)
>>>> id()[/color][/color]
> Traceback (most recent call last):
> File "<pyshell#1 >", line 1, in ?
> id()
> TypeError: 'IDGenerator' object is not callable[color=green][color=darkred]
>>>>[/color][/color][/color]

But i do appreciate your comment, thanks!

--
mph

[Martin P. Hellwig]

Re: This coding style bad practise?

keirr wrote:[color=blue]
> Martin P. Hellwig wrote:[color=green]
>> 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
>>[/color]
>
> 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.
>[/color]

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]

Re: This coding style bad practise?

Am Donnerstag 04 Mai 2006 01:04 schrieb Martin P. Hellwig:[color=blue]
> Because of:[color=green][color=darkred]
> >>>> id = IDGenerator("01 ",99)
> >>>> id()[/color]
> >
> > Traceback (most recent call last):
> > File "<pyshell#1 >", line 1, in ?
> > id()
> > TypeError: 'IDGenerator' object is not callable[/color]
>
> But i do appreciate your comment, thanks![/color]

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]

Re: This coding style bad practise?

Carl Friedrich Bolz wrote:[color=blue]
> Bruno Desthuilliers wrote:
>[color=green]
>> Martin P. Hellwig a écrit :
>>[color=darkred]
>>> 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?[/color]
>>
>>
>> Why not just use the call operator instead ? ie:
>>[color=darkred]
>> >>> id = IDGenerator(... )
>> >>> id()[/color]
>> 01_20060424_151 903_1[color=darkred]
>> >>> id()[/color]
>> 01_20060424_151 905_2[/color]
>
>
> because that shadows a builtin?[/color]

oops :(
[color=blue]
> sorry, could not resist :-)[/color]

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



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

[bruno at modulix]

Re: This coding style bad practise?

Martin P. Hellwig wrote:[color=blue]
> Bruno Desthuilliers wrote:
> <cut>
>[color=green]
>>
>> Why not just use the call operator instead ? ie:
>>[color=darkred]
>> >>> id = IDGenerator(... )
>> >>> id()[/color]
>> 01_20060424_151 903_1[color=darkred]
>> >>> id()[/color]
>> 01_20060424_151 905_2
>>[/color]
>
> Because of:
>[color=green][color=darkred]
>>>>> id = IDGenerator("01 ",99)
>>>>> id()[/color]
>>
>> Traceback (most recent call last):
>> File "<pyshell#1 >", line 1, in ?
>> id()
>> TypeError: 'IDGenerator' object is not callable
>>[color=darkred]
>>>>>[/color][/color][/color]

Of course - you have to overload the call operator for this to work.
Just rename IDGenerator.__r epr__ to IDGenerator.__c all__, 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 'onurb@xiludom. gro'.split('@')])"

[Martin P. Hellwig]

Re: This coding style bad practise?

bruno at modulix wrote:[color=blue]
> Martin P. Hellwig wrote:[color=green]
>> Bruno Desthuilliers wrote:
>> <cut>
>>[color=darkred]
>>> Why not just use the call operator instead ? ie:
>>>
>>> >>> id = IDGenerator(... )
>>> >>> id()
>>> 01_20060424_151 903_1
>>> >>> id()
>>> 01_20060424_151 905_2
>>>[/color]
>> Because of:
>>[color=darkred]
>>>>>> id = IDGenerator("01 ",99)
>>>>>> id()
>>> Traceback (most recent call last):
>>> File "<pyshell#1 >", line 1, in ?
>>> id()
>>> TypeError: 'IDGenerator' object is not callable
>>>[/color][/color]
>
> Of course - you have to overload the call operator for this to work.
> Just rename IDGenerator.__r epr__ to IDGenerator.__c all__, and I garantee
> this will work - and will be *much* more cleaner than abusing __repr__.
>
>
>[/color]
Thanks! That was the thing I was looking for!

--
mph

[Martin P. Hellwig]

Re: This coding style bad practise?

<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 -----[color=blue]
> import string
> import time
>
> class IDGenerator(obj ect):
> """(leading _id, subversion_leng th, 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_leng th.
>
> Example usage:[color=green][color=darkred]
> >>> id = IDGenerator('01 ',2)
> >>> id()[/color][/color]
> '01_20060504_11 2304_1'[color=green][color=darkred]
> >>> id()[/color][/color]
> '01_20060504_11 2306_2'[color=green][color=darkred]
> >>> id()[/color][/color]
> '01_20060504_11 2307_1'[color=green][color=darkred]
> >>>
> >>> id = IDGenerator(000 5,99) # Note that an int will be cast to a string!
> >>> id()[/color][/color]
> '5_20060504_112 324_01'[color=green][color=darkred]
> >>> id()[/color][/color]
> '5_20060504_112 327_02'[color=green][color=darkred]
> >>> id[/color][/color]
> <class '__main__.IDGen erator'> previous ID is 5_20060504_1123 24_01 and
> current ID is 5_20060504_1123 27_02[color=green][color=darkred]
> >>>[/color][/color]
>
> """
>
> def __init__(self,l eading_id, subversion_leng th, timezone='gm'):
> self.id = str(leading_id)
> self.length = int(subversion_ length)
> fill_length = len(str(self.le ngth))
> self.current = None
> self.previous = None
>
> def fill(number):
> return(string.z fill(number,fil l_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.le ngth+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(retur n_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.cur rent)
>
> def __repr__(self):
> return(str(self .__class__) +
> ' previous ID is ' +
> str(self.previo us) +
> ' and current ID is ' +
> str(self.curren t))[/color]
----- script -----