6
\$\begingroup\$

I have just gotten into Object Oriented Programming and I've been practicing it through Python. But I think I still haven't clearly grasped how it should be used effectively. I've created a game Using OOP where a random deck is generated and will be compared to a random number given to an NPC. Below are the source code:

import random
import os

class User:
def __init__(self, username, NumList):
    self = NPC()
    self.username = username
    self.NumList = NumList
    print(f'Welcome {self.username}! We have initialized your card!')
    print(f'Your numbers are: {self.NumList}')
    User.userSystem(self)

def userSystem(self):
    print("")
    print(f'So, What would you like to do now, {self.username}?')
    print(f'Type (1) to Reroll your card')
    print(f'Type (2) to bet')

    userInput = int(input())
    if userInput == 1:
        Deck.reRoll(self)
        User.userSystem(self)
    elif userInput == 2:
        Deck.bet(self)
    else:
        print("Invalid Argument! Please type a valid number!")
        print("")
        return User.userSystem(self)
    

class Deck(User):
def __init__(self, username):
    self.NumList = []
    for x in range(5):
        self.NumList.append(random.randrange(0,100))
    User.__init__(self, username, self.NumList)

def reRoll(self):
    print(f'Re-rolling your Numbers!')
    self.NumList = []
    for x in range(5):
        self.NumList.append(random.randrange(0,100))
    print('Numbers Re-rolled!')
    print(f'Your new numbers are: {self.NumList}')

def bet(self):
    print("")
    myCard = self.NumList
    print(f'Your cards are: {myCard}')
    print(f'The value is: {sum(myCard)}')
    print("")
    opponentCard = NPC.callNumbers(self)
    print(f'Your opponents cards are: {opponentCard}')
    print(f'Their value is: {sum(opponentCard)}')
    print("")

    if sum(myCard) > sum(opponentCard):
        print(f'Congratulations {self.username}! You Won!')
    elif sum(myCard) < sum(opponentCard):
        print(f'You lost the round, {self.username}!')
    else:
        print(f'It is a draw!')

class NPC(User):
def __init__(self):
    self.username = f'Player {random.randrange(1,1000)}'
    self.numlist = []
    for x in range(5):
        self.numlist.append(random.randrange(0,100))

def callNumbers(self):
    return self.numlist

#Initiate
user = input("Welcome! Please type your name: ")
os.system('CLS')
user = Deck(user)

I would love to know if this is how Object Oriented Programming supposed to be used, or in any case perhaps I am making a mistake or any unnecessary steps.

\$\endgroup\$
3
  • 4
    \$\begingroup\$ Can you fix your indentation? \$\endgroup\$
    – Linny
    Commented Aug 9 at 7:54
  • \$\begingroup\$ @Linny yeah I'm sorry, I kinda copy pasted it from my editor and the indentation broke out, but code-wise, how do you think it is? \$\endgroup\$ Commented Aug 9 at 8:34
  • \$\begingroup\$ Please follow the PEP 8 style guide when writing python code. Most of your identifiers should have been in snake_case. \$\endgroup\$
    – J_H
    Commented Aug 9 at 11:11

4 Answers 4

1
\$\begingroup\$

Indentation

I realize posting a question with proper formatting can be tricky for new users of Stack Exchange sites, but all the functions need to be indented by one level.

Portability

When I run the code, I get this error:

sh: CLS: command not found

The CLS command may work on some operating systems, but not on my Linux OS.

os.system('CLS')

I assume the intent is to clear the screen. Do some research to see if there is a more portable way to do this in Python.

Documentation

I do not understand anything about this game. You need to add a docstring at the top of the code to describe its purpose:

"""
This is a game where a player does blah-blah-blah...
"""

Also, each class should have a docstring describing its purpose. For example, if Deck is for a deck of cards, it should be explicitly stated. Also describe what type of cards and how many there are. I expect a deck to have 52 cards, but it looks like there are only 5 (like a hand of cards).

Naming

The PEP 8 style guide recommends using snake_case for function and variable names. For example, reRoll is better as re_roll, userSystem as user_system, etc.

Also userSystem is not a very descriptive name.

The term "roll" is not normally associated with cards; it is more commonly used with dice. "Deal" is more common with cards. If you stick with "roll", you should add more documentation to describe why.

Input

The code gracefully handles incorrect numeric input for userInput, but it dies badly with non-numeric input.

Plural

Change card to cards in the following display message:

    print(f'Welcome {self.username}! We have initialized your card!')
\$\endgroup\$
2
\$\begingroup\$

I would love to know if this is how Object Oriented Programming is supposed to be used

Not really. Currently, your Deck runs the game, and your User runs the bets.

Instead, a Deck should just be a deck object containing a list of card numbers (plus methods to manipulate those cards), and a User should just be a user object containing a name and deck (plus associated methods). Create a separate Game class to run the actual matches between user decks.


self.NumList = []
for x in range(5):
    self.NumList.append(random.randrange(0,100))

The idiomatic way to generate lists is using list comprehensions. It's also preferable to use snake_case for variables and to exclude the data structure from the name:

self.cards = [random.randrange(100) for _ in range(5)]

Also consider parameterizing these "magic" numbers (100 and 5) instead of hardcoding them.

\$\endgroup\$
1
\$\begingroup\$

If the intention is to practice OOP, first thing should be to think about what objects you will need, and let's think about this all conceptually.

You have a Deck, so that's great, but should a deck need a number list? And is a deck a subtype of a user? That will mean that the deck has a username, which seems a bit odd. A deck will need to hold on to a list of Cards, so let's start there. We'll have a class for the Card, and we know that a card has a suit and a rank. We get something like

class Card:
  def __init__(self, suit, rank) -> None:
     self.suit = suit
     self.rank = rank

class Deck:
  def __init__(self) -> None:
    self.cards = []
    for suit in ["spades", "hearts", "clubs", "diamonds"]:
      for rank in list(range(1, 10)) + ["T", "J", "Q", "K", "A"]:
        cards.append(Card(suit, rank))

You could here then have enums for the suits and ranks, you could use list comprehension to initialise the deck, you will likely want methods like shuffle and draw for the deck, etc. It's up to you and the program's needs.

You also know that you want to build a Game. A game will need a game loop. In the game, there will be a player and an opponent. If you were to create classes for these - which you might need need (again, up to you) - I would guess that you'd want to keep track of scores, and you will want to define win criteria.

Best of luck!

\$\endgroup\$
0
\$\begingroup\$

I would love to know if this is how Object Oriented Programming supposed to be used

No.

There are lots of errors here...

class User:
    def __init__(self, username, NumList):
        self = NPC()
        ...
        User.userSystem(self)

    def userSystem(self):
        # do something


class NPC(User):
    def __init__(self):
        # create instance

self is a reserved moniker in a class to represent a specific, initialised class instance.

For example User is a class, but user = User() is a class instance.

def userSystem(self): defines an instance method but you have called it as if it is defined as a class method. You have called it as User.userSystem rather than self.userSytem. When you call an instance method the self argument is passed automatically, you do not need to call self.instance_method(self) for example.

It doesn't look like you would need class methods here so best to avoid them, particularly in training.

This is also bad: self = NPC(). Since self is reserved to represent the class instance you are creating you are immediately overwriting this in the local scope and setting it to another class: NPC. I can't see this causes anything other than confusion and broken programs, and certainly serves no purpose. If the User class needs an NPC class then you should be using something like adding an instance attribute to the User:

self.npc = NPC()

This way the User class maintains its self moniker and has attached a new instance of the NPC class to itself (whatever NPC actually means).

\$\endgroup\$

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.