Best way to make a weighted sum

Steven Taschuk staschuk at telusplanet.net
Tue Apr 8 16:36:00 EDT 2003


Some comments on your code, and an alternative approach.

Quoth Jorge Godoy:
  [...]
> class IEModulo11:

I don't know why you're using a class for this.  A simple function
which calculates (and/or verifies) the checksum seems more natural.

Consider that, to calculate the checksum for a given value of ie
with your class, I have to do this:
	calculator = IEModulo11(ie)
	calculator.EstipulaPesos('RJ')
	calculator.CalculaSoma()
	# now sum is in calculator.soma
What a pain.  With a simple function for this purpose, it's just:
	sum = checksum(ie, 'RJ')

And, despite the class name, you don't actually do a mod 11.  I
assume this is a bug.

>     def __init__(self, ie=None, uf=None):
>         
>         self.estados = ['CE', 'AP', 'RJ'] 
>         self.ie = ie
>         self.soma = 0

Why isn't the uf argument used?

>     def EstipulaPesos(self, uf=None):
>       """
>       First digit indicates the number position, the second indicates
>       it's weight.
>       """
> 
>       if uf in ['CE', 'AP']:
>             self.pesos = {8:9, 7:8, 6:7, 5:6, 4:5, 3:4, 2:3, 1:2}
>       else:
>             if uf in ['RJ']:
>                 self.pesos = {7:2, 6:7, 5:6, 4:5, 3:4, 2:3, 1:2}

If uf is not in ['CE', 'AP', 'RJ'], then self.pesos simply does
not get set; this will cause CalculaSoma to fail later, in a
mysterious way.  Better to have an informative exception raised at
this point, surely.

It's also more natural imho to have the weights in a dict keyed by
uf.  That dict can replace your self.estados, which I note is not
otherwise used.

Also, the weights are conceptually a sequence, not a map; why not
make them a list?

>     def CalculaSoma(self):
> 
>         ie = self.ie
>         iter = len(ie)
> 
>         for i in ie:
>             produto = i * self.pesos[iter]
>             self.soma = self.soma + produto
>             iter = iter - 1

Why not return the sum?  See above.

It's also not good form to use 'iter' as a variable name, since
'iter' is the name of a built-in function.

> What I want is to sum the products of a certain 'ie' number where each
> of its digits is multiplied by a certain weigth, depending on its
> position inside the number. (This is for some documents verification
> and 'ie' can vary in the amount of digits as well as its weigth can
> vary from one place to another --- I know this is a mess, but it's the
> way some Brazilian states did it...)

An alternative:

	import operator

	weights = {}
	weights['CE'] = weights['AP'] = [9, 8, 7, 6, 5, 4, 3, 2]
	weights['RJ'] = [2, 7, 6, 5, 4, 3, 2] # is this right?

	def checksum(ie, state):
		w = weights[state][-len(ie):]
		return reduce(operator.add, [x*y for x, y in zip(ie, w)]) % 11

Imho this is much simpler, more straightforward, and easier to use.

-- 
Steven Taschuk                                     staschuk at telusplanet.net
Receive them ignorant; dispatch them confused.  (Weschler's Teaching Motto)





More information about the Python-list mailing list