Why does the third iteration of my code return incorrectly?

Viewed 58

I am in a MOOC for Python. This is my first time posting a question here.

My expected results should print Captain Hawk, Doctor Yellow Jacket, and Moon Moon, each on their own line.

Instead, I get Captain Hawk, Doctor Yellow Jacket, and Moon Yellow Jacket each on their own line. What is wrong with my code?


#A common meme on social media is the name generator. These
#are usually images where they map letters, months, days,
#etc. to parts of fictional names, and then based on your
#own name, birthday, etc., you determine your own.
#
#For example, here's one such image for "What's your
#superhero name?": https://i.imgur.com/TogK8id.png
#
#Write a function called generate_name. generate_name should
#have two parameters, both strings. The first string will
#represent a filename from which to read name parts. The
#second string will represent an individual person's name,
#which will always be a first and last name separate by a
#space.
#
#The file with always contain 52 lines. The first 26 lines
#are the words that map to the letters A through Z in order
#for the person's first name, and the last 26 lines are the
#words that map to the letters A through Z in order for the
#person's last name.
#
#Your function should return the person's name according to
#the names in the file.
#
#For example, take a look at the names in heronames.txt
#(look in the drop-down in the top left). If we were to call
#generate_name("heronames.txt", "Addison Zook"), then the
#function would return "Captain Hawk": Line 1 would map to
#"A", which is the first letter of Addison's first name, and
#line 52 would map to "Z", which is the first letter of
#Addison's last name. The contents of those lines are
#"Captain" and "Hawk", so the function returns "Captain Hawk".
#
#You should assume the contents of the file will change when
#the autograder runs your code. You should NOT assume
#that every name will appear only once. You may assume that
#both the first and last name will always be capitalized.
#
#HINT: Use chr() to convert an integer to a character.
#chr(65) returns "A", chr(90) returns "Z".


#Add your code here!

def generate_name(filename, name):    
    alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
    
    # splitting the name so I can add the intials of the name into a list.
    z = []
    j = name.split()
    for i in j:
        z.append(i[0])
    
    # linking the initials to it's index in the alphabet  
    for k in alphabet:
        if k == z[0]:
            first = alphabet.index(k)
        elif k == z[1]:
            second = alphabet.index(k) + 26
        else:
            pass
        
        
    # reading the super hero names from the file and linking the name to the index   
    file = open(filename, "r") 
    filelist = file.readlines()

    for i in filelist:
        global hero
        if filelist.index(i) == first:
            supe = i
            supe = supe.strip()
        elif filelist.index(i) == second:
            hero = i
            hero = hero.strip()
        else:
            pass
    
    file.close()
    return supe + " " + hero
        

       
        

#Below are some lines of code that will test your function.
#You can change the value of the variable(s) to test your
#function with different inputs.
#
#If your function works correctly, this will originally
#print: Captain Hawk, Doctor Yellow Jacket, and Moon Moon,
#each on their own line.
print(generate_name("heronames.txt", "Addison Zook"))
print(generate_name("heronames.txt", "Uma Irwin"))
print(generate_name("heronames.txt", "David Joyner"))

The file that has the superhero names is called heronames.txt and it looks like this, but every name is on it's own line:

Captain
Night
Ancident
Moon
Spider
Invisible
Silver
Dark
Professor
Golden
Radioactive
Incredible
Impossible
Iron
Rocket
Power
Green
Super
Wonder
Metal
Doctor
Masked
Crimson
Omega
Lord
Sun
Lightning
Knight
Hulk
Centurion
Surfer
Warriors
Ghost
Hornet
Yellow Jacket
Moon
Ghost
Phantom
Machine
X
Doom
Z
Fist
Shadow
Claw
Torch
Soldier
Skull
Thunder
Hurricane
Falcon
Hawk

Any insight will be greatly appreciated. Thank you!

3 Answers

if filelist.index(i) is your issue. You are saying with this code: "Tell me the index of the item in this list that has the value moon". It's going to return 3 because moon first appears in that position.

Because of that and the fact that you are doing if/elif only the first if condition ever fires, setting the supe twice and never setting the hero.

Lastly because you declare hero as a global, it retains the last hero value from the previous call and instead getting a NULL hero/second-name you get the previous hero's second name.

Instead of iterating the file, you already have determined the index of the item you need for first and second name, so replace

for i in filelist:
    global hero
    if filelist.index(i) == first:
        supe = i
        supe = supe.strip()
    elif filelist.index(i) == second:
        hero = i
        hero = hero.strip()
    else:
        pass

With

supe = filelist[first].strip()
hero = filelist[second].strip()

Your function is overly complicated. Here are a couple mistakes I see:

for k in alphabet:
    if k == z[0]:
        first = alphabet.index(k)
    elif k == z[1]:
        second = alphabet.index(k) + 26
    else:
        pass

You can just use for index, k in enumerate(alphabet) and then use index instead of searching alphabet for the letter k. You already know the index, since that is the string you're iterating over!

However, you don't need that loop at all. You know what letter you're looking for, so you might as well just use

first = alphabet.index(z[0])
second = alphabet.index(z[1]) + 26

Same thing with for i in fileList: You already know the index of i since you iterate over fileList, so just use enumerate. HOWEVER, you don't need to do that here either: You know the indices you want, so just select them:

supe = filelist[first].strip()
hero = filelist[second].strip()

Consider the following simplified code:

def generate_name(filename, name):
    alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
    z = [word[0].upper() for word in name.split(maxsplit=1)]
    first = alphabet.index(z[0])
    second = alphabet.index(z[1]) + 26
    with open(filename, "r") as f:
        filelist = f.readlines()
        
    supe = filelist[first].strip()
    hero = filelist[second].strip()
    return supe + " " + hero

Changes I made:

  • I used a list comprehension to create z. This basically condenses your for i in j loop into a single line. I also convert the character to uppercase just to be safe
  • I specify maxsplit=1 in name.split because we only want two values.
  • I used with to handle closing the file after we're done with it. This is more pythonic than opening a file handle and explicitly closing it.

Some other notes:

  • You don't need to define alphabet: Getting the index of the first character of either name is easily done by subtracting its ASCII code (ord) from the ASCII code for "A". e.g: first = ord(z[0]) - ord("A")
  • You can use the f-string syntax to format the return value. return f"{supe} {hero}"

I am not sure where your code goes wrong. You can simplify your code a bit, to make it easier to figure out where it is wrong. Your two for loops are not needed, as you can index your lists instead. For example:

    for k in alphabet:
    if k == z[0]:
        first = alphabet.index(k)
    elif k == z[1]:
        second = alphabet.index(k) + 26
    else:
        pass

Becomes

first = alphabet.index(z[0])
second = alphabet.index(z[1])

You don't need to loop through the alphabet as Z[i] always is part of the alphabet.

You can do something similar with your last loop.

Hope it helps.

Related