Generell återkoppling på students kod #47
Description
Återkoppling på all kod en student lämnade in i kursen, HT18.
Generellt
I kursen jobbar vi med relativt små problem och program därför kan det vara svårt att hitta strukturella problem med din kod. I denna kursen uppstår bara det vid stora och uppenbara fel. Generellt tycker jag att du har väldigt bra struktur på din kod och koden är skriven som vi vill att den ska se ut i denna kursen.
Jag kan ta upp saker som vi kanske inte direkt har pratat om i kursen så vissa saker kan tolkas som extra hårda eller du inte visste om. Även saker som vi själva gjort fel på genomgångar och i exempel.
Bra saker
Importerar specifika funktioner från moduler och inte hela modulen. Ex:
from random import randint, choice, random
while/if satsen i main.py är "ren" du gör bara funktionsanrop i den. Ex.
#Mask string
elif choice == "11":
m.maskString()
# Marvin status
elif choice == "12":
m.marvinStatus()
Kan förbättras
Multipla print() anrop
Flera print() för "ett" meddelande. Ex.
print("\nSnake says:\n")
print("Hello %s - your awesomeness!" % name)
print("What can I do you for?!")
Det är "dyrt" i prestanda att göra print(), helst vill du bara göra print en gång. Då behöver du bygga upp en stäng och sen skriva ut bara den. Ex.
msg = """
Snake says:
Hello {} - your awesomeness
What can i do you for?!""".format(name)
print(msg)
Jag skrev även om från % till .format() för att injecta värden i strängen då det är bättre. Jag såg att du använt format till det på andra ställen. %s, även kallat "printf-style string formatting", är det gammla sättet att formatera strängar. Men med Python3 introducerades .format() för att göra sträng formatering mer konsekvent, tydligare och mindre buggigt. %s är även nästan "deprecated". Med Python 3.6 har ytterligare ett nytt sträng formaterings sätt introducerats, f-Strings. Men vi är inte så moderna i kursen att vi behandlar det än.
kmom05
Globala variabl
Oftast vill man undvika globala variabler för de komplicerar snabbt koden och det blir lätt fel när man måste tänka på om man ska byta värden på dem. Men i din kod passar det att använda en global variable för filnamnet.
filename = "inv.data"
I och med att värdet bara kommer läsas och inte ändras kalla vi det en konstant. För att markera att det är en konstant namnger vi variabeln med stora bokstäver.
FILENAME = "inv.data"
Funktionsuppdelning
Man brukar säga att en funktion ska endast göra en sak och din writetofile() funktion är exempel där det kan förbättras.
def writetofile(items_as_list):
items_as_str = ",".join(items_as_list)
with open(filename, "w") as filehandle:
filehandle.write(items_as_str)
Nu har du ett litet program där du bara ska använda datan från filen på ett sätt men om vi tänker att det ska fungera för mer saker i framtiden så är det bättre om funktionen inte kräver att datan kommer som en lista och sen gör om den till en sträng. Pythons write() funktion klarar bara av strängar som argument så om du t.ex. även ska skriva en integer till filen måste du göra om den till en lista innan du kan skicka den till funktionen. Då är mer modulärt om du låter writetofile() ta emot en sträng medans en tidigare funktion ansvarar för att göra om datan från en annan typ till en sträng innan du anropar writetofile().
def list_to_string(items_as_list):
return ",".join(items_as_list)
def writetofile(items_as_string):
with open(filename, "w") as filehandle:
filehandle.write(items_as_str)
Kolla om en sträng är tom
För att kolla om en sträng är tom i din kod har du följande.
if items_as_str == "":
....
Ett mer pythonic sätt att göra det är följande.
KOD: MARKERA ALLT
if items_as_str:
....
En tom sträng motsvarar False vid booleska jämförelser.
Vilka andra sånna tolkningar som finns kan vi se i ett foruminlägg av en student som tar upp olika värdens booleska representation.
kmom06
Byta fil
När man byter vilken fil som ska användas i analyzer gillar jag att du gör en koll om filen finns eller inte.
if choice == "change":
tmp = a.change()
try:
f = open(tmp, "r")
f.close()
filename = tmp
except FileNotFoundError:
print("\nFilen du angav hittades inte.")
Men jag hade lagt den koden i en funktion och inte löst i if-satsen. Jag hade skrivit om den som följer, tar även med funktionen change() från analyzer.py.
if choice == "change":
filename = a.change()
....
def change():
new_name = input("\nSkriv in ett filnamn att analysera: ")
return check_if_file_exists(new_name)
def check_if_file_exist(filename):
try:
f = open(filename, "r")
f.close()
return filename
except FileNotFoundError:
print("\nFilen du angav hittades inte.")
return ""
Try1
Läsa fil
Detta fanns även i kmom06. För analyze delen öppnar du filen i varje funktion med with och jobbar med innehållet i det blocket.
def spaces(filename):
count = 0
with open(filename) as filehandle:
for line in filehandle:
for c in line:
if c == " ":
count += 1
print(count)
För att få mindre kod duplication kan du skriva en ny funktion som bara läser upp innehållet, returnerar det och sen anropa den i varje funktion istället. Nu gör det väldigt liten skillnad men det är något att tänka på.
def spaces(filename):
count = 0
lines = read_file(filename)
for line in lines:
for c in line:
if c == " ":
count += 1
print(count)
def read_file(filename):
with open(filename) as filehandle:
return filehandler.readlines()
På detta sättet får vi även ett block mindre, en "intabbning", i koden och då blir det lättare att läsa. Min lösning kan faktiskt vara långsammare än din för att vi måste läsa upp innehållet två gånger i min lösning. Men om vi nödvändigtvis hade velat ha en snabb lösning hade vi gjort på ett annat sätt.
Mutera lista i en loop
För uppgift 3 i try1 har du en kodsnutt som ändrar på en lista som loopas igenom.
for i, _ in enumerate(number_list):
if i % 2 == 0:
number_list[i] = number_list[i] * 2
if len(str(number_list[i])) > 1:
tmp = str(number_list[i])
number_list[i] = int(tmp[0]) + int(tmp[1])
Du loopar på number_list och i den gör du.
number_list[i] = number_list[i] * 2
Det är någon som man generellt ska undvika att göra. Om man gör det kan det ställa till buggar, speciellt om man försöker ta bort eller lägga till värden.
Istället vill vi skapa en ny lista som vi lägger till alla värden i istället.
result = []
for i, digit in enumerate(number_list):
if i % 2 == 0:
new_digit = digit * 2
if len(str(new_digit)) > 1:
tmp = str(new_digit)
new_digit = int(tmp[0]) + int(tmp[1])
result.append(new_digit)
else:
result.append(digit)