search

dbwebb-se / python

Coursematerial for python
Other
34 stars 33 forks source link

Återkoppling på students marvin4 #54

Closed AndreasArne closed 3 years ago

AndreasArne commented 3 years ago

Marvin4

search_country

Här kan vi snygga till if-satsen.

    if len(result_list) <= 0:
        raise ValueError("Country does not exist!")

Vi utnytjar vår kunskap om truthiness, tomma listor värderas som False.

    if not result_list:
        raise ValueError("Country does not exist!")

get_country_data

Vi kollar på if-selse satsen för att hantera population. Vi kan göra samma sak som ovanför, men också skippa att skapa variablerna pop1, pop2 och pop2.

    # om antal invånare saknas, används None
    if len(country_data["population"]) <= 0:
        pop1 = None
        pop2 = None
        pop3 = None
    else:
        pop1, pop2, pop3 = country_data["population"]
    pop_list = (pop1, pop2, pop3)

blir

    # om antal invånare saknas, används None
    if country_data["population"]:
        pop_list = country_data["population"]
    else:
        pop_list = (None, None, None)

Det finns ingen anledning att skapa pop variablerna när vi inte använder dem till något mer. I nästa steg av koden ser vi lite samma sak att du skapar en variabel bara för att på raden under bara tilldela värdet till något annat. Där tycker jag man kan skippa det. Notera dock att ibland är det bättre att först göra tilldelningen till en egen variabel som du har gjort. T.ex. om raden blir lång eller om man gör flera funktions anrop på rande, då är det tydligare att dela upp den som du har.

        emission = get_country_year_data_megaton(country, year)
        year_dict["emission"] = emission

blir

    year_dict["emission"] = get_country_year_data_megaton(country, year)

I slutet har du samma sak.

    em_change1 = get_country_change_for_years(country, year_list[0], year_list[1])
    em_change2 = get_country_change_for_years(country, year_list[1], year_list[2])
    em_change = (em_change1, em_change2)
    country_dict["emission_change"] = em_change

blir

    country_dict["emission_change"] = (
        get_country_change_for_years(country, year_list[0], year_list[1]),
        get_country_change_for_years(country, year_list[1], year_list[2])
    )

print_country_data

I denna funktionen kunde du gjort på samma sätt som du gjorde i förra med att ha en for-loop där du loopar över åren. Nu hårdkodar du mycket, vilket jag inte är helt emot i denna funktionen. Men om du ska hårdkoda kan du skippa att först lägga alla data i en lista. Vi kan börja med att kolla på lite smidigare kod för hur du har löst det. Du har:

def print_country_data(country):
    """
    skriver ut all tillgänglig data för angivet land
    """
    country_list = []

    # namn
    country_list.append(country["name"])

    # år 1990
    list_1990 = list(country["1990"].values())
    temp = ["1990"] + list_1990
    if temp[2] is None:
        temp[2] = "Missing population data!"
    country_list.append(temp)

    # år 2005
    list_2005 = list(country["2005"].values())
    temp = ["2005"] + list_2005
    if temp[2] is None:
        temp[2] = "Missing population data!"
    country_list.append(temp)

    # år 2017
    list_2017 = list(country["2017"].values())
    temp = ["2017"] + list_2017
    if temp[2] is None:
        temp[2] = "Missing population data!"
    country_list.append(temp)

    # emission change
    list_change = list(country["emission_change"])
    country_list.append(list_change)  

    # utskrift  
    print(country_list[0])
    print("Emission        - {a}: {ae} {b}: {be} {c}: {ce}"
            .format(a=country_list[1][0], 
                    ae=country_list[1][1],
                    b=country_list[2][0],
                    be=country_list[2][1],
                    c=country_list[3][0],
                    ce=country_list[3][1]))

    print("Emission change - {a}-{b}: {bc}% {b}-{c}: {cc}%"
            .format(a=country_list[1][0], 
                    b=country_list[2][0],
                    bc=country_list[4][0],
                    c=country_list[3][0],
                    cc=country_list[4][1]))

    print("Population      - {a}: {ap} {b}: {bp} {c}: {cp}"
            .format(a=country_list[1][0], 
                    ap=country_list[1][2],
                    b=country_list[2][0],
                    bp=country_list[2][2],
                    c=country_list[3][0],
                    cp=country_list[3][2]))

Första halvan har mycket kod duplicering och vi kan skriva om den till.

    for year in ("1990", "2005", "2017"):
        temp_list = [year] + list(country[year].values())
        if temp_list[2] is None:
            temp_list[2] = "Missing population data!"
        country_list.append(temp_list)

        country_list.append(list(country["emission_change"]))

I sista halvan hårdkodar du fram alla värden från listan vi skapade ovanför den kan vi också göra om till en for-loop på liknande sätt som jag gjorde ovanför. Jag visar inte kod för det.

Det jag ogillar med denna lösningen är att du har skrivit kod för att flytta in all data i en lista och sen har du hårkodat hur du använder datan i listan. Då tycker jag att vi lika gärna kan använda datan hårdkodat från början. Vi skippar att lägga in det i en lista och istället använder bara dictionaryn som skickas in som argument.

Då kan vi ha kod som ser ut typ så här istället:

    print(
        f"\t\t\t{country['name']}\n"
        f"Emission          - 1990: {country['1990']['emission']}\t2005: {country['2005']['emission']} \t2017: {country['2017']['emission']}\n",
        f"Emission change   -   1990-2005: {country['emission_change'][0]}% \t2005-2017: {country['emission_change'][1]}%",
    )
    if country['1990']['population']:
        print(
            f"Population        - 1990: {country['1990']['population']}\t2005: {country['2005']['population']} \t2017: {country['2017']['population']}",
        )
    else:
        print("Missing population data!")

Men denna koden kan vi också skriva om lite med en for-loop.

    print(
        f"\t\t\t{country['name']}\n"
    )
    for year, data in country:
        print(f"Emission          - {year}: {data['emission']}")

    print(f"Emission change   -   1990-2005: {country['emission_change'][0]}% \t2005-2017: {country['emission_change'][1]}%")

    if country['1990']['population']:
        for year, data in country:
            print(
                f"Population        - {year}: {data['population']}\n",
            )
    else:
        print("Missing population data!")

Kanske inte den snyggaste koden, men det blir gjort med lite kod och vi gör inget onödigt.