4
\$\begingroup\$

I'm currently learning python with an udemy course. I wrote this code, which is meant to be a higher lower game, where the second answer always stays. As far as i tested it my code is working fine, but i feel like many solutions are bad. Its just felt weird coding some of the solutions, but i cant come up with better stuff. I would really aprecciate if someone would oversee my code and give some example for bad writing of myself with a better solution.

Heres my code:

from art import logo, vs
import random 
from game_data import data
from replit import clear

def game():
  score = 0
  print(logo)
  lost = False
  options = ["spaceholder", random.choice(data)]
  options = print_options(options[1])
  while not lost:
    return_values = compare(options[0], options[1], score)
    score = return_values[1]
    lost = return_values[0]
    if not lost:
      options = print_options(options[1])


# print both persons
def print_options(a):
  char1 = a
  new_list = data
  new_list.remove(char1)
  char2 = random.choice(data)
  new_list.append(char1)
  print(f"Compare A: {char1['name']}, a {char1['description']}, from {char1['country']} ")
  print(vs)
  print (f"Against B: {char2['name']}, a {char2['description']}, from {char2['country']} ")
  return char1, char2


# get input and compare both persons
def compare(adict, bdict, score):
  guess=input("""Who has more followers? Type 'A' or 'B': """)
  a = adict["follower_count"]
  b = bdict["follower_count"]
  if guess == "A" and a > b:
    clear()
    print(logo)
    print(f"Youre right! Current score: {score + 1}")
    return False, score +1
  elif guess == "B" and b > a:
    clear()
    print(logo)
    print(f"Youre right! Current score: {score + 1}")
    return False, score + 1
  else:
    clear()
    print(logo)
    print(f"Sorry, thats wrong. Final score: {score}")
    return True, score



game()

Edit: imports: clear is just clearing the console art:

logo = """
    __  ___       __             
   / / / (_)___ _/ /_  ___  _____
  / /_/ / / __ `/ __ \/ _ \/ ___/
 / __  / / /_/ / / / /  __/ /    
/_/ ///_/\__, /_/ /_/\___/_/     
   / /  /____/_      _____  _____
  / /   / __ \ | /| / / _ \/ ___/
 / /___/ /_/ / |/ |/ /  __/ /    
/_____/\____/|__/|__/\___/_/     
"""

vs = """
 _    __    
| |  / /____
| | / / ___/
| |/ (__  ) 
|___/____(_)
"""

game data:

data = [
    {
        'name': 'Instagram',
        'follower_count': 346,
        'description': 'Social media platform',
        'country': 'United States'
    },
    {
        'name': 'Cristiano Ronaldo',
        'follower_count': 215,
        'description': 'Footballer',
        'country': 'Portugal'
    },
    {
        'name': 'Ariana Grande',
        'follower_count': 183,
        'description': 'Musician and actress',
        'country': 'United States'
    },
    {
        'name': 'Dwayne Johnson',
        'follower_count': 181,
        'description': 'Actor and professional wrestler',
        'country': 'United States'
    },
    {
        'name': 'Selena Gomez',
        'follower_count': 174,
        'description': 'Musician and actress',
        'country': 'United States'
    },
    {
        'name': 'Kylie Jenner',
        'follower_count': 172,
        'description': 'Reality TV personality and businesswoman and Self-Made Billionaire',
        'country': 'United States'
    },
    {
        'name': 'Kim Kardashian',
        'follower_count': 167,
        'description': 'Reality TV personality and businesswoman',
        'country': 'United States'
    },
    {
        'name': 'Lionel Messi',
        'follower_count': 149,
        'description': 'Footballer',
        'country': 'Argentina'
    },
    {
        'name': 'Beyoncé',
        'follower_count': 145,
        'description': 'Musician',
        'country': 'United States'
    },
    {
        'name': 'Neymar',
        'follower_count': 138,
        'description': 'Footballer',
        'country': 'Brasil'
    },
    {
        'name': 'National Geographic',
        'follower_count': 135,
        'description': 'Magazine',
        'country': 'United States'
    },
    {
        'name': 'Justin Bieber',
        'follower_count': 133,
        'description': 'Musician',
        'country': 'Canada'
    },
    {
        'name': 'Taylor Swift',
        'follower_count': 131,
        'description': 'Musician',
        'country': 'United States'
    },
    {
        'name': 'Kendall Jenner',
        'follower_count': 127,
        'description': 'Reality TV personality and Model',
        'country': 'United States'
    },
    {
        'name': 'Jennifer Lopez',
        'follower_count': 119,
        'description': 'Musician and actress',
        'country': 'United States'
    },
    {
        'name': 'Nicki Minaj',
        'follower_count': 113,
        'description': 'Musician',
        'country': 'Trinidad and Tobago'
    },
    {
        'name': 'Nike',
        'follower_count': 109,
        'description': 'Sportswear multinational',
        'country': 'United States'
    },
    {
        'name': 'Khloé Kardashian',
        'follower_count': 108,
        'description': 'Reality TV personality and businesswoman',
        'country': 'United States'
    },
    {
        'name': 'Miley Cyrus',
        'follower_count': 107,
        'description': 'Musician and actress',
        'country': 'United States'
    },
    {
        'name': 'Katy Perry',
        'follower_count': 94,
        'description': 'Musician',
        'country': 'United States'
    },
    {
        'name': 'Kourtney Kardashian',
        'follower_count': 90,
        'description': 'Reality TV personality',
        'country': 'United States'
    },
    {
        'name': 'Kevin Hart',
        'follower_count': 89,
        'description': 'Comedian and actor',
        'country': 'United States'
    },
    {
        'name': 'Ellen DeGeneres',
        'follower_count': 87,
        'description': 'Comedian',
        'country': 'United States'
    },
    {
        'name': 'Real Madrid CF',
        'follower_count': 86,
        'description': 'Football club',
        'country': 'Spain'
    },
    {
        'name': 'FC Barcelona',
        'follower_count': 85,
        'description': 'Football club',
        'country': 'Spain'
    },
    {
        'name': 'Rihanna',
        'follower_count': 81,
        'description': 'Musician and businesswoman',
        'country': 'Barbados'
    },
    {
        'name': 'Demi Lovato',
        'follower_count': 80,
        'description': 'Musician and actress',
        'country': 'United States'
    },
    {
        'name': "Victoria's Secret",
        'follower_count': 69,
        'description': 'Lingerie brand',
        'country': 'United States'
    },
    {
        'name': 'Zendaya',
        'follower_count': 68,
        'description': 'Actress and musician',
        'country': 'United States'
    },
    {
        'name': 'Shakira',
        'follower_count': 66,
        'description': 'Musician',
        'country': 'Colombia'
    },
    {
        'name': 'Drake',
        'follower_count': 65,
        'description': 'Musician',
        'country': 'Canada'
    },
    {
        'name': 'Chris Brown',
        'follower_count': 64,
        'description': 'Musician',
        'country': 'United States'
    },
    {
        'name': 'LeBron James',
        'follower_count': 63,
        'description': 'Basketball player',
        'country': 'United States'
    },
    {
        'name': 'Vin Diesel',
        'follower_count': 62,
        'description': 'Actor',
        'country': 'United States'
    },
    {
        'name': 'Cardi B',
        'follower_count': 67,
        'description': 'Musician',
        'country': 'United States'
    },
    {
        'name': 'David Beckham',
        'follower_count': 82,
        'description': 'Footballer',
        'country': 'United Kingdom'
    },
    {
        'name': 'Billie Eilish',
        'follower_count': 61,
        'description': 'Musician',
        'country': 'United States'
    },
    {
        'name': 'Justin Timberlake',
        'follower_count': 59,
        'description': 'Musician and actor',
        'country': 'United States'
    },
    {
        'name': 'UEFA Champions League',
        'follower_count': 58,
        'description': 'Club football competition',
        'country': 'Europe'
    },
    {
        'name': 'NASA',
        'follower_count': 56,
        'description': 'Space agency',
        'country': 'United States'
    },
    {
        'name': 'Emma Watson',
        'follower_count': 56,
        'description': 'Actress',
        'country': 'United Kingdom'
    },
    {
        'name': 'Shawn Mendes',
        'follower_count': 57,
        'description': 'Musician',
        'country': 'Canada'
    },
    {
        'name': 'Virat Kohli',
        'follower_count': 55,
        'description': 'Cricketer',
        'country': 'India'
    },
    {
        'name': 'Gigi Hadid',
        'follower_count': 54,
        'description': 'Model',
        'country': 'United States'
    },
    {
        'name': 'Priyanka Chopra Jonas',
        'follower_count': 53,
        'description': 'Actress and musician',
        'country': 'India'
    },
    {
        'name': '9GAG',
        'follower_count': 52,
        'description': 'Social media platform',
        'country': 'China'
    },
    {
        'name': 'Ronaldinho',
        'follower_count': 51,
        'description': 'Footballer',
        'country': 'Brasil'
    },
    {
        'name': 'Maluma',
        'follower_count': 50,
        'description': 'Musician',
        'country': 'Colombia'
    },
    {
        'name': 'Camila Cabello',
        'follower_count': 49,
        'description': 'Musician',
        'country': 'Cuba'
    },
    {
        'name': 'NBA',
        'follower_count': 47,
        'description': 'Club Basketball Competition',
        'country': 'United States'
    }
]
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Welcome to Code Review! Could you tell us about the imports, or better yet, show us that code too? \$\endgroup\$ Commented Nov 29, 2021 at 20:43
  • 3
    \$\begingroup\$ @200_success I added it. I hope thats what you meant And thanks for welcoming me here. Im very new to python btw \$\endgroup\$ Commented Nov 29, 2021 at 20:53

2 Answers 2

3
\$\begingroup\$

I'm not going to go too heavily into refactoring the whole thing.

Some basic readability things:

return_values = compare(options[0], options[1], score)
score = return_values[1]
lost = return_values[0]

It would be much clearer if you did:

lost, score = compare(options[0], options[1], score)

You have indented things with two spaces, which is odd, normally we use 4 spaces for indenting each level!

In terms of best practice:

print_options does a couple of things, both selecting another contestant AND printing them out. each function should only do one thing e.g.:

options = print_options(options[1])

could be: (I made some notes too..)

def get_options(char1): #a->char1
  #char1 = a #removed
  new_list = data
  new_list.remove(char1)
  char2 = random.choice(data) # Should be random.choice(new_list)?
  #new_list.append(char1) #This does nothing? 
  return char1, char2

def print_options(options):

    print(f"Compare A: {char1['name']}, a {char1['description']}, from {char1['country']} ")
    print(vs)
    print (f"Against B: {char2['name']}, a {char2['description']}, from {char2['country']} "

[...]     
    options = get_options(options[1])
    print_options(options)

The following is the same code copied and pasted, this is a good clue you could refactor it.

if guess == "A" and a > b:
    clear()
    print(logo)
    print(f"Youre right! Current score: {score + 1}")
    return False, score +1
elif guess == "B" and b > a:
    clear()
    print(logo)
    print(f"Youre right! Current score: {score + 1}")
    return False, score + 1

something along the lines of

if a>b and guess == "A" or a<b and guess == "B":
    clear()
    print(logo)
    print(f"Youre right! Current score: {score + 1}")
    return False, score + 1

Also, compare does at least three different things too (Get input, compare, and print) It might be good to split this into two or three functions (get_guess() check_guess(options, guess) and print_result(lost,score) might be a good idea.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Pssst, to make your answer even better (It is already great!), maybe mention something about the formatting of the code? 2 indents is really uncommon! ^^ \$\endgroup\$ Commented Nov 30, 2021 at 19:58
  • 2
    \$\begingroup\$ I hadn't noticed that, I wondered why I ended up with 6 spaces in mine... \$\endgroup\$
    – JeffUK
    Commented Nov 30, 2021 at 20:03
  • \$\begingroup\$ Thanks a lot for you answer! \$\endgroup\$ Commented Dec 1, 2021 at 11:59
0
\$\begingroup\$

Great tips from the other answer, here are just some more bits and bobs.

Use the library capabilities to their fullest

  char1 = a
  new_list = data
  new_list.remove(char1)
  char2 = random.choice(data)
  new_list.append(char1)

This, in addition to the uncommon indentation sticks out to me. Why are we removing, why are we adding, why are we redefining? Consider

char1, char2 = random.sample(data, 2)

Excplicit over implisitt

Try to make your code as clear as possible. We are not collecting data, we are collecting twitter data. Similarly we are not returning char1 and char2 but we are returning celebrities (or really accounts, but too lazy to change my code now)

def compare(adict, bdict, score):
  guess=input("""Who has more followers? Type 'A' or 'B': """)
  a = adict["follower_count"]
  b = bdict["follower_count"]

This also seems strange, why not just make compare take in a and b. Remember the thing about explicit? Yeah, those should probably not be called adict and bdict.

Tricks

What would you do if you needed to compare 3 or 4 twitter accounts? You would quickly run into issues. The quick and dirty solution is as follows:

We sort the accounts in decending order of number of followers

twitter_account_0, twitter_account_1

Then we label them with letters

twitter_account_0, twitter_account_1
        A                 B

The trick now is that we shuffle the labels instead of the actual accounts

twitter_account_0, twitter_account_1
        B                 A

So twitter_account_0 is really the biggest one. But we print them by label so it looks like

A, twitter_account_1
B, twitter_account_0

So when you type B, we just need to check that this corresponds to the account with index 0. Slightly quicker to do this with a lookup table. One very rough implementation looks like the following

import string

from art import logo, vs
import random
from game_data import twitter_data

TOTAL_CELEBRITIES = 3


def celebrity_info(celebrities_by_label, info):
    for label, celebrity in celebrities_by_label.items():
        print(label, end=", ")
        for key in info[:-1]:
            print(celebrity[key], end=", ")
        print(celebrity[info[-1]], end="\n\n")


def get_celebritites(twitter, total=TOTAL_CELEBRITIES, sort_key="follower_count"):

    indexes = random.sample(range(0, total), total)
    index_2_label = [string.ascii_uppercase[i] for i in indexes]
    label_2_index = {label: index for index, label in enumerate(index_2_label)}

    labels = list(string.ascii_uppercase[0:TOTAL_CELEBRITIES])

    celebrities = random.sample(twitter, total)
    celebrities.sort(key=lambda d: d[sort_key], reverse=True)
    celebrities_by_label = {
        label: celebrities[label_2_index[label]] for label in labels
    }
    return (
        celebrities_by_label,
        label_2_index,
        labels,
    )


# get input and compare both persons
def round(celebrities_by_label, label_2_index, labels):
    celebrity_info(celebrities_by_label, ["name", "description"])

    answer = input(f"Who has more followers? Type one of {labels}: ").upper()
    # The celebrity with index 0 always has the highest count
    return label_2_index.get(answer, None) == 0


def game():
    print(logo)
    celebrities_by_label, label_2_index, labels = get_celebritites(
        twitter_data, TOTAL_CELEBRITIES
    )

    points = 0
    while round(celebrities_by_label, label_2_index, labels):
        points += 1
        print(f"Youre right! Current score: {points}")
        celebrities_by_label, label_2_index, labels = get_celebritites(
            twitter_data, TOTAL_CELEBRITIES
        )
    print(f"Sorry, thats wrong. Final score: {points}\n")

    celebrity_info(celebrities_by_label, ["name", "description", "follower_count"])


if __name__ == "__main__":

    game()

The proper way

The proper way is to make a class that is a datastructure and let each twitter account be an instance of this class. I will leave it to you to give this a chance. It is a great opportunity to get introduced to object oriented programming!

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the Answer! It helped a lot and i am going to look into OOP today. \$\endgroup\$ Commented Dec 1, 2021 at 11:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.