kmom02/marvin/a5 - dåliga vanor(?) och effektivisering

[jf-Lindberg]

Under arbetet med kmom02 kändes mina lösningar till vissa av uppgifterna forcerade och lite onaturliga. Även om lösningarna funkar och jag inte får några valideringsfel känns det som det går att förbättra dem. Jag hamnar lite i att "släcka bränder" ibland känns det som. Det här rör främst menyvalen i marvin och framför allt menyval a5.

Koden finns uppladdad här: https://umbridge.arnesson.dev/results/inspect/565038/BYYLmHb3PvUxfdFP6hYtIz0XY2tEzCFKM35x3VRt

Menyval A5 (förklaring följer nedan):

point_string = input('Input a string: ') # "a2b4A5s3B1"
final_str = ""
used_letters = ""
length = len(point_string)
lowercase = point_string.lower()

for letter in lowercase:
    result = 0
    if letter in used_letters or letter.isdigit():
        continue
    used_letters += letter
    if letter.isalpha():
        for i, char in enumerate(point_string):
            if char == letter and i < (length - 1):
                if point_string[i+1].isdigit():
                    point = int(point_string[i+1])
                result += point
            elif char == letter.upper() and i < (length -1 ):
                if point_string[i+1].isdigit():
                    point = int(point_string[i+1])
                result -= point
    final_str += letter + " " + str(result) + ", "
print(final_str[:-2])

Koden ovan funkar och passerar alla tester, men det känns definitivt som att den kan förbättras ordentligt, framför allt när det kommer till poängräknandet. De två if-statementsen som kollar om point_string[i+1] har ett numeriskt värde känns inte naturliga, men jag får ValueErrors om jag inte kör dem. Jag har också funderingar på ifall variabeln used_letters är bästa sättet att hålla koll på vilka bokstäver som inte ska räknas med fortsättningsvis - det känns inte som det. Provade flera andra lösningar men detta var den som funkade utan några problem.

Uppskattar all feedback möjlig på denna uppgift. Jag satt rätt länge med den så jag skulle vilja se vart jag tänkt fel och vad jag kunnat göra annorlunda. Nojjar lite över att komma in i ett "släcka bränder"-tänk där jag lägger till mer kod för att lösa problem jag skapat av att skriva dålig kod från början.

[AndreasArne]

Återkoppling

Det är vanligt med "släcka bränder" tänk nu i början. Oftast skulle jag säga att det är bättre att ta bort kod eller börja om från början när man känner att det börjar inträffa. Om du börjar om från början har du mer kunskap än när du började förra gången och kan få till en bättre grund.

Jag har redan gett feedback på A5 ett par gånger tidigare, det går att hitta dem här om man är intresserad #48 och #49. Men det skadar ju inte med feedback på en till lösning.

Du har rätt bra kod, du använder två for-loopar på ett bra sätt där du väljer en bokstav och räknar ut alla dess poäng. Men vi kan städa upp den lite.

I första for-loopen har du if letter.isalpha(): vilket inte behövs i och med att du redan ovanför den gör:

if letter in used_letters or letter.isdigit():
    continue

Då kan inte letter vara något annat än alpha.

Nästa sak är i < (length - 1) i if char == letter and i < (length - 1):. Jag förstår varför du har lagt dit den och du har troligen behövt det i ett tidigare stadie i din kod men som koden ser ut nu tillför det inget. I början av if-satsen kollar du att char är lika med letter och om det är sant kan inte i vara lika eller större än length. i är bara det om char är den sista siffran och då är inte char lika med letter. Så den koden kollar efter något som aldrig kommer inträffa. Istället räcker det med:

if char == letter:
    ...
elif char == letter.upper():
    ...

Då kan vi också ta bort length = len(point_string) från början.

Som fortsättning på att vi förutsätter att point_string har en specifik struktur och alltid har vartannat bokstav och siffra kan vi också ta bort if point_string[i+1].isdigit():. Om if char == letter: blir sant vet vi att point_string[+1] kommer innehålla en siffra och då behöver vi inte kolla om det är en siffra.

Sen tycker jag att din kod ser bra ut. Nu har vi blivit av med all onödig kod och resterande är en passande lösning på uppgiften. Den sista biten koden som vi tog bort hade kunnat vara kvar om testerna var elakare. Vi kunde ta bort den bara för att vi vet att input strängen alltid har samma struktur men om det fanns några tester med strängar som inte har korrekt struktur då hade vi behövt den koden.

Jag hade också gjort om:

point = int(point_string[i+1])
result -= point

Till en rad.

result -= int(point_string[i+1])

Här tycker jag inte att det tillför så mycket att lägga heltalsvärdet i en egen variabel. I och med att vi endast använder variabeln för att subtrahera på raden under. Om vi skulle använt värdet till något mer hade jag lagt den i en egen variabel dock.

All kod:

point_string = input('Input a string: ') # "a2b4A5s3B1"
final_str = ""
used_letters = ""
lowercase = point_string.lower()

for letter in lowercase:
    result = 0
    if letter in used_letters or letter.isdigit():
        continue
    used_letters += letter
    for i, char in enumerate(point_string):
        if char == letter:
            result += int(point_string[i+1])
        elif char == letter.upper():
            result -= int(point_string[i+1])
    final_str += letter + " " + str(result) + ", "
print(final_str[:-2])

[jf-Lindberg]

Tack så mycket för superbra feedback, @AndreasArne!