0

Before I even try to explain this, here is the code I'm working with currently:

currentLine = 1
try:
    with open(filename, 'r') as f:
        for line in f:
            if currentLine == 1:
                tempName = line
            if currentLine == 2:
                tempColour = line
            if currentLine == 3:
                tempAge = line
            if currentLine == 4:
                tempWeight = line
                currentLine = 1
                tempSheep = Sheep(tempName, tempColour, tempAge, tempWeight)
                sheepList.append(tempSheep)
            currentLine = currentLine + 1
except:
    print("file is in an invalid format")
    continue
else:
    break

The goal of the code is to read 4 line from a file (name, colour, age, and weight) and put them into a Sheep object. This needs to be done in a loop, as there are anywhere from 2 - 10 sheep per file. The code mostly works, in that it reads lines and puts them in the class, but it doesn't read the right lines. When I print out all the sheep, every sheep as the same name, "bob", which is the name of the first sheep in the file and is the first line. Beyond that, it actually works, but it totally ignores the name variable, just putting bob in it. I end up with this mess of sheeps named bob.

for example, a sample output looks like this:

Name: bob
Colour: blue
age: 5
weight: 50

name: bob
Colour: tina
age: red
Weight: 7

name: bob
colour: 75
age: shirley
Weight: green

in case it isn't obvious, its offsetting everything by ignoring the name. I hope this was explained well enough, if you need further explanation I can try to put in some more samples.

why are my program be bad?

Kukumi
  • 1
  • 2

2 Answers2

4
        if currentLine == 4:
            tempWeight = line
            currentLine = 1
            tempSheep = Sheep(tempName, tempColour, tempAge, tempWeight)
            sheepList.append(tempSheep)
        currentLine = currentLine + 1

When this if block executes, it initially sets currentLine to one. Then currentLine = currentLine + 1 executes, setting it to 2. This means that, by the time you reach the top of the loop again, the if currentLine == 1: check will never succeed.

Try setting currentLine to zero instead.

        if currentLine == 4:
            tempWeight = line
            currentLine = 0
            tempSheep = Sheep(tempName, tempColour, tempAge, tempWeight)
            sheepList.append(tempSheep)
        currentLine = currentLine + 1

... But you might be better off skipping the if blocks entirely. If each of your records is exactly four lines long, you can extract each one using one of the recipes from How do you split a list into evenly sized chunks?. Then you can pass the data to the Sheep constructor via argument unpacking.

def chunks(l, n):
    """Yield successive n-sized chunks from l."""
    for i in range(0, len(l), n):
        yield l[i:i + n]

with open(filename) as f:
    lines = [line.strip() for line in f]

sheepList = []
for group in chunks(lines, 4):
    sheepList.append(Sheep(*group))
Kevin
  • 74,910
  • 12
  • 133
  • 166
  • I like the chunks approach, I haven't seen that implemented outside of itertools before, +1 – C.Nivs Apr 02 '19 at 15:07
1

The business of keeping track of your file lines is making things a bit tricky. You can use enumerate to make things a bit easier and just use the modulo operator % to group things by 5 (each set of names is in groups of 5 lines):

for i, line in enumerate(f, start=1): # Start numbering at 1
    currentLine = i % 5 # At line 5, blank, this will be zero
    if not currentLine: # 0 acts as False
        continue

    # if elif else makes things a bit more efficient
    # as it's not possible for you to have multiple line number
    # possibilities
    if currentLine == 1:
       tempName = line
    elif currentLine == 2:
       tempColour = line
    elif currentLine == 3:
       tempAge = line
    else:
       tempWeight = line
       tempSheep = Sheep(tempName, tempColour, tempAge, tempWeight)
       sheepList.append(tempSheep)

C.Nivs
  • 12,353
  • 2
  • 19
  • 44
  • `enumerate` is definitely preferable over manually incrementing a counter. There's almost always a better alternative to doing `something += 1` on your own :-) – Kevin Apr 02 '19 at 15:05