Funktion verändern ohne Redundanz?

[Judge]

Hallo zusammen!

Ich entwickle gerade mit einer 3rd party library (spotipy). Diese hat eine Funktion `prompt_for_user_token()`, in der eine weitere Funktion aufgerufen wird, allerdings mit einem geringen Anteil an hartcodierter Vorgabe. Ich habe den relevanten Teil mal ausgeschnitten:

Code: Alles auswählen

def prompt_for_user_token(username, scope=None, client_id = None,
        client_secret = None, redirect_uri = None, cache_path = None):
    sp_oauth = oauth2.SpotifyOAuth(client_id, client_secret, redirect_uri, 
        scope=scope, cache_path=".cache-" + username)
    token_info = sp_oauth.get_cached_token()
    code = sp_oauth.parse_response_code(response)
    token_info = sp_oauth.get_access_token(code)
    # Auth'ed API request
    if token_info:
        return token_info['access_token']

Ich habe das etwas zusammengekürzt. Mein Problem mit dieser Funktion ist nun, das ich den Parameter `cache_path` der Funktion `oauth2.SpotifyOAuth()` gerne frei setzen würde; wie man sieht wird dieser hier durch einen hartcodierten Anteil ".cache-" forciert.
Da es sich um eine 3rd party Library handelt, möchte ich ungern den Quellcode verändern, damit man nicht das altbekannte Update- und Partabilitäts-Problem hat.
Nun handelt es sich bei der Funktion um eine recht simple, welche man ohne weiteres kopieren, verändern und stattdessen verwenden könnte, allerdings schafft man so code-Redundanz.

Ich habe mich gefragt ob man sowas nicht mit einer mir bisher unbekannten Technik nicht eleganter lösen kann. Korrigiert mich wenn ich falsch liege, aber mit Decoratorn z.B. "wrapped" man ja nur die ganze Funktion und kann nicht irgendwas aus der Mitte heraus anders machen, oder?

[__blackjack__]

Also am ”saubersten” würde mir hier nur einfallen eine eigene Klasse für `username` zu schreiben die `__radd__()` implementiert um das addieren zu ”verhindern”.

Code: Alles auswählen

class MagicUsername(object):
    
    def __init__(self, username):
        self.username = username
    
    def __radd__(self, other):
        assert other == '.cache-'
        return self.username

Schön ist natürlich anders und man muss da vielleicht noch mehr implementieren wenn `username` in der Funktion noch für mehr verwendet wird.

[Sirius3]

Wenn das das spotipy ist, dann sieht der Abschnitt der Funktion in Wirklichkeit so aus:

Code: Alles auswählen

    cache_path = cache_path or ".cache-" + username
    sp_oauth = oauth2.SpotifyOAuth(client_id, client_secret, redirect_uri, 
        scope=scope, cache_path=cache_path)

dass also nur `".cache-" + username` genommen wird, wenn kein `cache_path` übergeben wird.

[Judge]

Also am ”saubersten” würde mir hier nur einfallen eine eigene Klasse für `username` zu schreiben die `__radd__()` implementiert um das addieren zu ”verhindern”.

Hey, __blackjack__ - danke für die Idee! __radd__() kenne ich nicht - lese ich mir gleich mal an was sich genau dahinter verbirgt.

Wenn das das spotipy ist, dann sieht der Abschnitt der Funktion in Wirklichkeit so aus:

Code: Alles auswählen

    cache_path = cache_path or ".cache-" + username
    sp_oauth = oauth2.SpotifyOAuth(client_id, client_secret, redirect_uri, 
        scope=scope, cache_path=cache_path)

dass also nur `".cache-" + username` genommen wird, wenn kein `cache_path` übergeben wird.

Hey Sirius3,

jajain:
1. "Ja": Das ist Spotipy, korrekt.
2. "Ja": Der Codeteil sieht im GitHub Repo inzwischen so aus wie Du sagst, ja.
3. "Nein": Der sah noch nicht so aus als ich mit der Entwicklung begonnen habe und komischerweise ist das, was man über PyPi als aktuellste ziehen kann auch noch so wie von mir beschrieben; bei gleicher Versionsnummer. Keine Ahnung ob da ein Release nicht sauber gelaufen ist oder was auch immer.
4. Mit dem Code, wie er im Git Repo (https://github.com/plamere/spotipy/blob ... py/util.py) aktuell ist, würde sich das Problem in der Tat nicht mehr darstellen. Allerdings ist diese Frage nach wie vor grundsätzlich interessant für mich, ob man in derart gestaltete Funktionen elegant eingreifen kann oder nicht.

[__blackjack__]

Allgemein ist die Antwort eher Nein, denn eine Funktion sollte ja eigentlich eine „blackbox“ sein, mit einer definierten Schnittstelle nach aussen und irgendwelche Magie die man nicht kennen muss, innen drin. Wenn der Autor nicht vorgesehen hat da irgendwie eingreifen oder injizieren zu können, dann gibt's keinen eleganten Weg das doch zu tun.

[Judge]

Allgemein ist die Antwort eher Nein, denn eine Funktion sollte ja eigentlich eine „blackbox“ sein, mit einer definierten Schnittstelle nach aussen und irgendwelche Magie die man nicht kennen muss, innen drin. Wenn der Autor nicht vorgesehen hat da irgendwie eingreifen oder injizieren zu können, dann gibt's keinen eleganten Weg das doch zu tun.

Alles klar - das hilft mir in Zukunft sicher, da ich mir nicht ständig, wenn ich eine solche Funktion copy&paste um sie marginal zu verändern, ein schlechtes Gewissen machen muss

[__blackjack__]

Wenn man das ständig macht, sollte man aber schon überlegen was da nicht stimmt, denn dann programmiert man gegen die offizielle API von Bibliotheken.

[__deets__]

Alles klar - das hilft mir in Zukunft sicher, da ich mir nicht ständig, wenn ich eine solche Funktion copy&paste um sie marginal zu verändern, ein schlechtes Gewissen machen muss

Genau dieses schlechte Gewissen solltest du haben. So zu arbeiten macht dich massiv anfaellig dafuer, dass dein Code zerbroeselt wie ein Zwieback in der Gesaesstasche, weil du eine neue Version einer Abhaengigkeit benutzt (willentlich oder nicht) - und ploetztlich kracht es. So vorzugehen ist nur in absoluten Ausnahmefaellen angezeigt, und auch nur vernuenftig kontrollierbar, wenn man dann die Versionen der Abhaengigkeiten bis auf's i-tuepfelchen kennt und installiert (zB durch die requirements.txt).

[Sirius3]

Gerade bei OpenSource ist der richtige Weg, zu versuchen, die gewünschte Änderung ins Projekt zurückzuspielen (was im konkreten Fall scheinbar schon passiert ist). Ich würde im Zweifel eher mit einer pre-Release-Version arbeiten, als irgendwo etwas hineinzupatchen.