sebelk
sebelk

Reputation: 644

Only get the last pair of dictionary in python3

I have a script that read and parse a file that has lines as follows, every field is separated by ",":

Ezequiel Garay,23,230,0,1

I made a function to obtain the first field that has the maximum value resulting of: (4th_field * 3rd_field ) / 2nd_field

My code is as follows:

#!/usr/bin/python3
def maxperf(lj):
    n = lj[0]
    if (int(lj[1])) > 0:
            performance=((int(lj[3]) + 1)*(int(lj[2])))/int(lj[1]) 
    else: performance=0

    par = dict([(n,performance)])    
    return max(par, key = par.get)

archivojugs = open("datos_ej1.txt")
jugadores = []
for jugador in archivojugs:
    jugadores = jugador.split(",")
    mp=maxperf(jugadores) 


print(mp)
archivojugs.close()

The problem is that I get only the last pair of the dictionary (I mean, it's as if each line overwrite the earlier one instead of append it), what's wrong with my code?

UPDATE: I've modified the answers:

#!/usr/bin/python3
def calcperf(n,pj,d,gg):
    '''Calculo performance'''
    if int(pj) > 0:
        p = ((int(gg) + 1 ) * int(d)) / int(pj)
    else: p = 0

    return  p

def maxperf(lp):
    '''Saco el jugador con mejor rendimiento'''
    mp = max(lp, key=lp.get)
    return mp



archivojugs = open("datos_ej1.txt")
listperfs = {}

for jugador in archivojugs.readlines():
    NOMBRE, PJ, DISTANCIA, GOLES, CONTROL = jugador.split(',')
    rendimiento = calcperf(NOMBRE,PJ,DISTANCIA,GOLES)
    listperfs[NOMBRE] = rendimiento

mejorperf = maxperf(listperfs)

print(mejorperf)

archivojugs.close

And it works fine

Upvotes: 1

Views: 70

Answers (1)

Michał Góral
Michał Góral

Reputation: 1517

That's because you're printing mp, which changes in each iteration of your for loop and you only get its state from the last iteration. Also, your par dictionary is local to maxperf function and contains only one entry each time you call that function.

You need need a non-local dictionary which would store results of your maxperf function. Let's create one which doesn't store computed performance at all, just returns it:

def perf(lj):
    n = lj[0]
    if (int(lj[1])) > 0:
        performance=((int(lj[3]) + 1)*(int(lj[2])))/int(lj[1]) 
    else:
        performance=0
    return n, performance

Now, back to our loop:

par = {}
for jugador in archivojugs:
    jugadores = jugador.split(",")
    name, p = perf(jugadores)
    par[name] = p

maxperf = max(par, key=par.get)
print(maxperf)

Also, keep in mind that storing performances in dictionary, just for sake of finding maximum value, is unnecessary and you'd be better with something like this:

import operator
mp = max(perf(jugador.split(',')) for jugador in archivojugs,
         key=operator.itemgetter(1))
print(mp)

Upvotes: 2

Related Questions