a
, dict
, c
and sum
are all problematic names, for two different reasons. Single character variable names should be used sparingly, really only in brief contexts where the intent is quite clear like for i in range(10)
where it's obvious that i
is an integer and follows for
loop conventions. for y in a
is very unclear because you haven't made it obvious what y
is or should be, and a
would be much harder to follow if not for the fact that it's declared just above. Even so the choice of letters seems totally arbitrary. I'd replace a
with f
for file, since it's localised.
The other naming issue is that using the names dict
and sum
are actually overwriting builtin functions and should be avoided, especially as I'm going to suggest that you use sum
, which you can't do if it's been overwritten.
Also instead of using a plain open
, you should use with open()
. That's Python's context manager which handles file closing for you. It ensures that a file is always closed correctly, even if an error arises or you don't call file.close()
. Note, you didn't call a.close()
so clearly this is useful:
with open('/home/Username/1st few programms/names.txt') as f:
for line in f:
names = line.split(",")
names.sort()
Next, this code you're using makes no sense. You're iterating over each line
in the file but just creating a new list of names each time. If all the names are stored on one line then this works but is redundant, and I'd instead suggest this:
with open('/home/Username/1st few programms/names.txt') as f:
names = f.read().split(",")
names.sort()
read()
will just read the full string of the file in so that then split
can separate it based on commas. If you do have names on multiple lines, then you want the loop but you want to add more values to names
each time:
names = []
with open('/home/Username/1st few programms/names.txt') as f:
for line in f:
names += line.split(",")
names.sort()
The algorithm that determines a person's score is quite unusual, so it would actually be best to create a function for it. One that takes a name and position in the list and returns their score. It can do largely what your inner loop does:
def name_score(index, name):
localsum = 0
for character in name:
localsum += char_points[character]
return localsum * (index + 1)
However, this is one of the places I'm going to suggest you use sum
. You can pass a generator expression into sum
to do this calculation efficiently in one line. A generator expression is a for loop like expression, here's how it'd look for this function:
def name_score(index, name):
return sum(char_points[character] for character in name) * (index + 1)
You can see how there's the same for _ in
syntax as before, just that now it's collapsed into one line. You no longer need to have a localsum
and sum
is faster than using a for
loop.
Now with this function, the main loop is simplified to:
sum = 0
for i in range(len(names)):
sum += name_score(i, names[i])
print sum
First though, let's get rid of names[i]
as that's ugly. Use Python's enumerate
function that allows you to both get the indices and values of a list together, like this:
for i, name in enumerate(names):
sum += name_score(i, name)
This is much cleaner now, but we can also use sum
in this case to remove your whole loop, in fact it can skip straight to printing the sum:
print sum(name_score(i, name) for i, name in enumerate(names))
Again, this is both more readable and much faster. This also means that you no longer have a sum
value that needs to be renamed.
names.txt
in? \$\endgroup\$