Modeling a soccer team with substitutions I'm new to Python and want to make sure I'm not developing bad habits. If you could please review this code below and give me any tips, practice ideas or critiques you might have. Any ideas on which aspects of the language I should focus on early would also be appreciated.

#!usr/env/python
#Modeling a soccer team with substitutions

#Players stored in dictionary {Name:Numbers}
players = {}
# Player positions stored in a list
forward = []
midfield = []
defense = []
goalie = []

class Team(object):
    def __init__(self, name):
        self.name = name


class Players(object):
    def __init__(self):
        pass
#creates a player list with names, numbers, and preferred positions
    def addPlayer(self, name, number, position):
        self.name = name
        self.number = number
        self.position = position
        players[name] = number

        if position == 'forward':
            forward.append(name)
            return forward
        elif position == 'midfield':
            midfield.append(name)
            return midfield
        elif position == 'defense':
            defense.append(name)
            return defense
        elif position == 'goalie':
            goalie.append(name)
            return goalie
        else:
            print 'Only forward, midfield, defense, goalie accepted'

        print "Added %s to the team!" % name


#Should list the players currently on the field. Only 3 player sample data
players_on_field = []
class OnField(object):
        def __init__(self):
            pass
        def sub_in(self, sub_name):
            self.sub_name = sub_name
            if len(players_on_field) == 3:
                print "Field is full, sub_out first!"
                #testing some options
                #x = raw_input('Which player would you like to sub out?')
                #OnField.sub_out(x)
            else:
                players_on_field.append(sub_name)
                print "%s entering the game" % sub_name
            return

        def sub_out(self, sub_name):
            for i in range(3):
                if players_on_field[i] == sub_name:
                    del players_on_field[i]
                    print "%s has been removed from the game" % sub_name
                    return
                #testing some options
                #x = raw_input('Who would you like to sub in?')
                #OnField.sub_in(x)
            else:
                print "That player is not on the field"
                return

        def list_players_on_field(self):
            print players_on_field



#Testing
Barcelona = Team('Barcelona')
this = Players()
game = OnField()

this.addPlayer('Messi', 10, 'forward')
this.addPlayer('Neymar', 11, 'forward')
this.addPlayer('Song', 9, 'midfield')
this.addPlayer('Valdes', 1, 'goalie')
this.addPlayer('Pique', 25, 'defense')
this.addPlayer('Pedro', 8, 'forward')

print players

print 'Forward Players'
print forward
print 'Midfield Players'
print midfield
print 'Defense Players'
print defense
print 'Goalies'
print goalie

#Subsitution testing
print "Players currently on the field"
print players_on_field
game.sub_in('Messi')
game.sub_in('Pedro')
game.sub_in('Song')
game.list_players_on_field()
game.sub_out('Pedro')
print players_on_field
game.sub_in('Neymar')
print players_on_field
You naming convention does not follow PEP 8. Try to follow it as much as you can as it helps having same consistent guidelines all over Python code.



I do not quite understand how the on field thingy is supposed to work so I'll skip on that part.



Try to avoid global variables. Try to think how things would be affected if you were trying to handle multiple teams. Most probably things would go wrong.

I guess your different lists should be part of the team class. Also, there is not much point for having a Player class as a player on its own is nothing but a name. As far as I understand soccer, the number is for a given player in a given team. Thus, the team should be some kind of containers of pairs name/number.

Also, it seems like your are doing the same things for different teams of players that could be handled the same way. Instead of write code, it's easier to use smart data structures and make them work for you. In your case, it seems like a dictionnary from position to list of (player name + player number) is good idea. Default composition is to have empty lists and to fill them later on.

Here's what my code looks like :

class Team(object):
    def __init__(self, name):
        self.name = name
        self.composition = {
            'forward' : [],
            'midfield': [],
            'defense': [],
            'goalie': []
        }

    def add_player(self, name, number, position):
        if position not in self.composition:
            print 'Only %s accepted' % ', '.join(self.composition.keys())
        else:
            self.composition[position].append((name,number))
            print 'Added %s to the team!' % name

bcn = Team('Barcelona')
bcn.add_player('Messi', 10, 'forward')
bcn.add_player('Neymar', 11, 'forward')
bcn.add_player('Song', 9, 'midfield')
bcn.add_player('Valdes', 1, 'goalie')
bcn.add_player('Pique', 25, 'defense')
bcn.add_player('Pedro', 8, 'forward')
bcn.add_player('Josay', 10, 'invalid')

# It could be a good idea to create a method to do this :-)    
for pos, players in bcn.composition.iteritems():
    print pos
    for p in players:
        print p