Prośba o krytykę jakości napisanego kodu

2015-01-09 03:03
0

Cześć. Od razu mówię, że proszę o poświęcenie mi trochę czasu, więc jeśli ktoś nie ma ochoty pomagać to niech nie pomaga i proszę o wyrozumiałość - chcę się czegoś nauczyć. Wstawiam krótki kod w języku Python. Ponieważ dopiero się go uczę i nie znam go na wylot, to bardzo prosiłbym o jakieś wskazówki odnośnie kodu. Kod to rozwiązanie zadania z konkursu programistycznego
sprzed kilku lat stąd dziwne nazwy jak Bajtazar, bo takie imię było w poleceniu do zadania. Z góry dziękuję za poświęcony mi czas :)

treść zadania jeśli ktoś chce ("AUTOMAT"): http://amppz.mimuw.edu.pl/amppz2012-tresci-zadan.pdf
kod: http://pastebin.com/Ex34eE66

class Bar:
    pass

class Choice:
    pass

#file with input data
tmpFile = open("Pyfood.txt", "r")

barsKinds, BajtazarsMoney = [int(x) for x in tmpFile.readline().split(" ")]
barsPrices = [int(x) for x in tmpFile.readline().split(" ")]
barsAmount = [int(x) for x in tmpFile.readline().split(" ")]

barsList = []

for x in range(barsKinds):
    newBar = Bar()
    newBar.price = barsPrices[x]
    newBar.amount = barsAmount[x]
    barsList.append(newBar)

def findBestOneToBuy(barsList):
    """ Finds best bar to buy (with biggest profit) and returns this as a structure Choice """
    bestChoice = Choice()
    bestChoice.idx = 0 
    bestChoice.profit = 0

    for x in range(len(barsList)): #check every possible condition
        tmpChoice = Choice()
        tmpChoice.idx = x
        tmpChoice.profit = 0

        if (barsList[x].amount == 0 or
            barsList[x].price > BajtazarsMoney) : continue 

        moneySaved = 0
        moneySpent = 0

        for i in range(x):  # for every possible condition count profit
            print(i)
            if barsList[i].amount > 0 : moneySaved += barsList[i].price

        moneySaved += barsList[x].price
        moneySpent = barsList[x].price
        tmpChoice.profit = moneySaved/moneySpent

        if tmpChoice.profit > bestChoice.profit : bestChoice = tmpChoice
        print(tmpChoice.profit)

    return bestChoice

def buyBar(idx, barsList):
    """ Takes idx of bar you want to buy and decreases amounts of 
    bars in barsList. returns price to pay and totalValue of stolen goods
    as a tuple """
    totalValue = 0
    for bar in range(idx):
        if barsList[bar].amount >0 :
            barsList[bar].amount -= 1
            totalValue += barsList[bar].price

    totalValue += barsList[idx].price
    barsList[idx].amount -= 1
    return barsList[idx].price, totalValue

moneyGained = 0
while True:
    barToBuy = findBestOneToBuy(barsList)
    print("chosen", barToBuy.idx+1)
    if barsList[barToBuy.idx].price > BajtazarsMoney : break

    moneySpent, tmpMoney = buyBar(barToBuy.idx, barsList)

    BajtazarsMoney -= moneySpent
    moneyGained += tmpMoney

print(moneyGained)

głównie zależy mi na wskazówkach typu:
"to i to jest nieczytelne"
"tego i tamtego nie powinno się używać" etc.

Mam nadzieję, że wiecie o co mi chodzi. Niestety nie mam nikogo, kto mógłby mi na miejscu dawać takie rady, więc męczę was. Jeszcze raz dziękuję :)

wklejenie kodu do posta - @furious programming

edytowany 1x, ostatnio: furious programming, 2015-01-09 03:46

Pozostało 580 znaków

2015-01-09 10:38
1

1) Formatowanie. Przeczytaj PEP 8 (zwłaszcza "Pet Peeves")

https://www.python.org/dev/peps/pep-0008/

2) Nazewnictwo:

tmpFile = open("Pyfood.txt", "r")

Dlaczego "tmp"?

3) Klasa-nie klasa

W funkcji findBestOneToBuy zwracasz niby-klasę (obiekt z przypisanymi na biegu atrybutami).
Ale w funkcji buyBar zwracasz już parę liczb. Skąd różnica? Umiesz uzasadnić?


Szacuje się, że w Polsce brakuje 50 tys. programistów
edytowany 1x, ostatnio: vpiotr, 2015-01-09 10:40
tmpFile było takie, ponieważ chciałem potem zrezygnować z tego, żeby dane były wprowadzane z pliku (wczytywanie z pliku skaracało mi czas sprawdzania poprawności programu), ale tego nie zrobiłem, a nazwy zmiennej nie poprawiłem, jakoś mi to umknęło, mój błąd. Jeżeli chodzi o różnicę w tym że raz zwracam tę klasę, a raz krotkę, to zrobiłem tak, ponieważ chciałem użyć tej niby klasy cos na wzór struktury z języka C (gdzieś w jakimś tutorialu czytałem, że tak się robi), dlatego nie chciałem dodawać nowych pól do klasy, ale już wiem, że nie powinienem tego w ogóle robić. Dzięki - Dawidpi 2015-01-09 22:28

Pozostało 580 znaków

2015-01-09 12:59
2

Punkt ode mnie: NIE rób tak klas! Serio! Jak piszesz klasę to zrób w niej normalny konstruktor i wylistuj tam pola obiektu. Takie "dodawanie" pól do obiektu na gorąco co prawda "działa", ale w większym projekcie to byś osiwiał jakbyś w ten sposób pracował...


Masz problem? Pisz na forum, nie do mnie. Nie masz problemów? Kup komputer...

Pozostało 580 znaków

2015-01-09 14:29
2

Najważniejsze jest chyba to, co napisał @Shalom.

Ode mnie nazewnictwo zmiennych i funkcji. Używasz camelCase i nie jest to oczywiście błąd, ale w Pythonie przyjęło się zapisywać je w taki sposób: foo_bar.
https://www.python.org/dev/pe[...]escriptive-naming-conventions

Pozostało 580 znaków

Liczba odpowiedzi na stronę

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0