Noch ein ISBN-Modul

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
Benutzeravatar
pixewakb
User
Beiträge: 1407
Registriert: Sonntag 24. April 2011, 19:43

Ich weiß, dass es hiervon schon eine Menge an Tools, Skripten, Modulen usw. gibt. Das hier war eher ein Pausenfüller. Ich wäre an Kritik interessiert, was könnte man besser machen und wo mache ich noch gravierende Fehler.

Für Hinweise und Kritik bin ich offen:

Code: Alles auswählen

def checker(isbn):
    '''
    Rudimentaere Pruefung, ob die ISBN
    korrekt sein koennte. Pruefung erfolgt
    rein formal:
    - Ist die ISBN lang genug (10 oder 13 Stellen)?
    - Stimmt die Pruefziffer (PZ)?
    '''
    isbn = str(isbn)
    
    if len(isbn) == 13 and isbn == isbn[:12] + isbn13PZ(isbn[:12]):
        return True
    elif len(isbn) == 10  and isbn == isbn[:9] + isbn10PZ(isbn[:9]):
        return True
    else:
        return False


def isbn10to13(isbn10):
    '''
    Bekommt die 10stellige ISBN und gibt
    die 13stellige ISBN als String zurueck
    '''
    isbn13 = "978" + isbn10[:9] + isbn13PZ("978" + isbn10[:9])   
    
    return isbn13


def isbn10PZ(isbn9):
    '''
    Bekommt die 9stellige ISBN und gibt die
    Pruefziffer (PZ) als String zurück
    '''
    zwErg = 0
    for i in range(0,9):

        zwErg += int(isbn9[i]) * (i + 1)

    if zwErg % 11 == 0:
        return "X"
    else:
        return str(zwErg % 11)


def isbn13to10(isbn13):
    '''
    Bekommt die 10stellige ISBN und gibt
    die 13stellige ISBN als String zurueck
    '''
    return isbn13[3:-1] + isbn10PZ(isbn13[3:-1])   


def isbn13PZ(isbn12):
    '''
    Bekommt die 12stellige ISBN und gibt die
    Pruefziffer (PZ) als String zurück
    '''
    summe = []
    
    for i in range(12):
        if (i + 1) % 2 == 0:
            summe.append(int(isbn12[i]) * 3)
        else:
            summe.append(int(isbn12[i]))

    return str((10 - (sum(summe) %  10)) % 10)
Zuletzt geändert von Anonymous am Mittwoch 4. Januar 2012, 08:01, insgesamt 1-mal geändert.
Grund: Quelltext in Python-Code-Tags gesetzt.
BlackJack

@pixewakb: Ein paar Namen sind nicht PEP8-konform. Bei `zwErg` muss ich immer lächeln, aber ich halte Fabelwesen für keine guten Namen für Zwischenergebnisse. ;-) Funktionsnamen sollten eher Tätigkeiten statt „Dinge“ beschreiben, also `check` statt `checker`.

`checker` könnte man mit einem ``return`` schreiben, welches einfach nur das Ergebnis der Bedingungsauswertung zurück gibt. Ein explizites zurück geben eines literalen Wahrheitswertes als einzige Reaktion auf eine Bedingungsauswertung ist IMHO ein „code smell“.

Die Umwandlungsfunktionen enthalten Wiederholungen, die ich aus den Ausdrücken heraus ziehen würde.

Statt ``for i in range(obj):`` um dann `i` für den Indexzugriff auf `obj` zu verwenden, kann man in Python direkt über die Elemente iterieren. Wenn man zusätzlich noch den Index benötigt, gibt es die `enumerate()`-Funktion.

Beide Prüfziffernberechnungen sind recht „langatmig“ beschrieben. Das lässt sich deutlich kürzer formulieren.

Der Name `summe` für eine Liste ist irreführend. Ich sehe auch nicht warum Du die Zwischenergebnisse in einer Liste sammelst, und nicht gleich summierst!?

Ungetestet:

Code: Alles auswählen

def check(isbn):
    '''
    Rudimentaere Pruefung, ob die ISBN
    korrekt sein koennte. Pruefung erfolgt
    rein formal:
    - Ist die ISBN lang genug (10 oder 13 Stellen)?
    - Stimmt die Pruefziffer (PZ)?
    '''
    isbn = str(isbn)
    return (
        len(isbn) == 13 and isbn[-1] == isbn13pz(isbn[:12])
        or len(isbn) == 10 and isbn[-1] == isbn10pz(isbn[:9])
    )


def isbn10to13(isbn10):
    '''
    Bekommt die 10stellige ISBN und gibt
    die 13stellige ISBN als String zurueck
    '''
    new_isbn = '978' + isbn10[:9]
    return new_isbn + isbn13pz(new_isbn)


def isbn10pz(isbn9):
    '''
    Bekommt die 9stellige ISBN und gibt die
    Pruefziffer (PZ) als String zurück
    '''
    result = sum(int(c) * (i + 1) for i, c in enumerate(isbn9)) % 11
    return 'X' if result == 0 else str(result)


def isbn13to10(isbn13):
    '''
    Bekommt die 10stellige ISBN und gibt
    die 13stellige ISBN als String zurueck
    '''
    new_isbn = isbn13[3:-1]
    return new_isbn + isbn10pz(new_isbn)


def isbn13pz(isbn12):
    '''
    Bekommt die 12stellige ISBN und gibt die
    Pruefziffer (PZ) als String zurück
    '''
    result = sum(
        (c if i % 2 == 0 else c * 3) for i, c in enumerate(map(int, isbn12))
    )
    return str((10 - result % 10) % 10)
Benutzeravatar
pixewakb
User
Beiträge: 1407
Registriert: Sonntag 24. April 2011, 19:43

`checker` könnte man mit einem ``return`` schreiben, welches einfach nur das Ergebnis der Bedingungsauswertung zurück gibt. Ein explizites zurück geben eines literalen Wahrheitswertes als einzige Reaktion auf eine Bedingungsauswertung ist IMHO ein „code smell“.
Weil man diese Prüfung im normalen Programm erledigen könnte? Man könnte diese Funktion wohl noch erweitern. Danke für die Kritik.

Wird etwas dauern, bis ich alles gelesen und mir angeeignet habe. Einige Spracheigenschaften oder -konstrukte sind mir noch nicht geläufig.
Benutzeravatar
/me
User
Beiträge: 3554
Registriert: Donnerstag 25. Juni 2009, 14:40
Wohnort: Bonn

BlackJack hat geschrieben:

Code: Alles auswählen

def isbn10pz(isbn9):
    '''
    Bekommt die 9stellige ISBN und gibt die
    Pruefziffer (PZ) als String zurück
    '''
    result = sum(int(c) * (i + 1) for i, c in enumerate(isbn9)) % 11
    return 'X' if result == 0 else str(result)
Als alternative Variante würde ich hier folgende kleinere Änderung vorschlagen:

Code: Alles auswählen

    result = sum(int(c) * i for i, c in enumerate(isbn9, start=1)) % 11
BlackJack

@/me: Braucht aber ein Python 2.7. Ich beschränke mich halt immer auf das was ich in der Praxis auf Servern vorfinde und das ist in der Mehrheit in meinem Fall noch 2.6.
lunar

@BlackJack: Aus der Dokumentation:
Changed in version 2.6: The start parameter was added.
BlackJack

Interessant, der DocString verschweigt das:

Code: Alles auswählen

In [55]: enumerate?
Type:           type
Base Class:     <type 'type'>
String Form:    <type 'enumerate'>
Namespace:      Python builtin
Docstring:
    enumerate(iterable) -> iterator for index, value of iterable                
    
    Return an enumerate object.  iterable must be another object that supports
    iteration.  The enumerate object yields pairs containing a count (from
    zero) and a value yielded by the iterable argument.  enumerate is useful
    for obtaining an indexed list: (0, seq[0]), (1, seq[1]), (2, seq[2]), ...


In [56]: list(enumerate('abc', 1))
Out[56]: [(1, 'a'), (2, 'b'), (3, 'c')]
Antworten