4
\$\begingroup\$

I just started to use classes in Python, so I don't know all concepts, rules and conventions about it. I made a little program of a machine gun which simulates firing by user's action. bullets is somehow the stock, and magazine, bullets ready to fire, in the gun. When magazine is at 0, you reload it with bullets. I'm using python 3.7. Here it is:

import random, pygame, sys

class MachineGun(object):
    bullets = 200

    def __init__(self):
        self._magazine = 50

    def _get_magazine(self):
        return self._magazine

    def _set_magazine(self, val):
        self._magazine = val

    magazine = property(_get_magazine, _set_magazine)

    def fire(self):
        pygame.mixer.Sound("fire.wav").play()
        while pygame.mixer.get_busy():
            pass

        fired = random.randint(1, 21)
        self.magazine = self.magazine-fired

        if self.magazine <= 0 :
            print("Bullets remaining : 0")
        else :
            print("Bullets remaining : {}".format(self.magazine))

    def reload(self):
        if self.magazine == 50 :
            print("Full")

        else :
            if MachineGun.bullets >= 50 :
                if self.magazine == 0 :
                    self.magazine = 50
                    MachineGun.bullets = MachineGun.bullets-50
                else :
                    tmp = 50-self.magazine
                    self.magazine = self.magazine+tmp
                    MachineGun.bullets = MachineGun.bullets-tmp

            else :
                if self.magazine == 0 :
                    self.magazine = MachineGun.bullets
                    MachineGun.bullets = 0
                else :
                    if self.magazine+MachineGun.bullets > 50 :
                        tmp = 50 - self.magazine
                        self.magazine = self.magazine+tmp
                        MachineGun.bullets = MachineGun.bullets-tmp
                    else :
                        self.magazine += MachineGun.bullets
                        MachineGun.bullets = 0

        pygame.mixer.Sound("reload.wav").play()
        while pygame.mixer.get_busy():
            pass

    def action(self):
        a = "x"

        while a != "e" :
            if a == "" :
                if self.magazine > 0 :
                    self.fire()
                elif self.magazine <= 0 and MachineGun.bullets > 0 :
                    self.magazine = 0
                    print("Reload")
                else :
                    print("End")
                    sys.exit()

            elif a == "r" :
                self.reload()

            a = input()

M = MachineGun()
M.action()

This is somehow my first "complete" implementation using classes. So I don't know if the form is correct. For exemple, I modified the attribute bullets without using a class method. Should I ? Like properties for object attributs ? Is it very necessary to use properties ?

\$\endgroup\$
0

2 Answers 2

Reset to default
2
\$\begingroup\$

UX

When I run the code, it just hangs in my shell. The code has this line:

a = input()

This means that the code is waiting for user input, but there is no indication of that. You should add text to the prompt which provides instructions for the user. For example:

a = input('Enter an option. To exit hit e, to reload hit r, to show remaining hit Enter ')

Also, a is not a very descriptive name for a variable in this context. action would be more appropriate. Since the function is already named action, I recommend renaming the function as run_game.

Import

It is a good practice to split each import onto its own line. Change:

import random, pygame, sys

to:

import random
import pygame
import sys

OOP

Using object is no longer needed:

class MachineGun(object):

It is simpler as:

class MachineGun():

You should structure the class such that you don't use the class name itself within the class. For example, it would be better if you did not use MachineGun in the line below:

if MachineGun.bullets >= 50 :

Generally, you should reference a class variable with self.

f-string

The following line:

print("Bullets remaining : {}".format(self.magazine))

is simpler with an f-string:

print(f"Bullets remaining : {self.magazine}")

Documentation

The PEP 8 style guide recommends adding doctrings to describe your class and functions. It would be nice to add a doctrings to summarize the purpose of the code as well. This header docstring would also contain instructions for the user as well as notifying the user that input .wav files are needed to run the code.

Whitespace

The code uses inconsistent whitespace. The black program can be used to automatically format the code consistently.

Assignment

This line:

MachineGun.bullets = MachineGun.bullets-50

can be simplified using a special assignment operator:

MachineGun.bullets -= 50

There are other instances of this. In fact, you already used it here:

self.magazine += MachineGun.bullets
\$\endgroup\$
1
\$\begingroup\$

Your reload function can be improved a bit:

def is_reload_required(self):
    return not self.magazine == 50

def reload(self):
    if not self.is_reload_required():
        print("Magazine is already full")
        # reload is not needed, so return from here
        return

    bullets_remaining = min(MachineGun.bullets, 50)
    bullets_needed_for_reload = bullets_remaining - self.magazine
    self.magazine += bullets_needed_for_reload
    MachineGun.bullets -= bullets_needed_for_reload

    pygame.mixer.Sound("reload.wav").play()
    while pygame.mixer.get_busy():
        pass

Also, you should not use magic numbers, for example, 50 for magazine capacity. Define named variables instead, like MAGAZINE_CAPACITY and assign it to 50. There also may be a way to make such variables as read-only constants (see documentation for @property: https://docs.python.org/3/library/functions.html#property).

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.