Ist es guter Stil, Klassen nur für eine Instanze zu machen ?

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
Septias
User
Beiträge: 80
Registriert: Freitag 24. Juni 2016, 19:15

Ich persönlich mag es nicht, Klassen zu erstellen, nur um davon dann ein Objekt zu erstellen. Deswegen habe ich dann einfach die Namensräume der Klassen so verwendet:

Code: Alles auswählen

class Config:
    noiseDistance = 30
    rotationX = 0
    rotationY = 0
    heightScale = 30
    _layers = 3

    @property
    @classmethod
    def layers(cls):
        return cls._layers

    @layers.setter
    def setter(cls, value):
        cls._layers = value
        
    noiseCount = 50
    
    @classmethod
    def valSetter(cls, val):
        def f(x):
            cls.__dict__[val] = x
        return f
Jetzt habe ich versucht, eine Property für meine Klasse anzulegen, aber als Classmethod funktioniert es nicht wirklich... habe alles Mögliche ausprobiert, aber man bekommt mit dem Aufruf Config.layers immer nur das Propertyobjekt an sich zurück, nicht aber den Wert.
Wie es scheint sind die Properties nur für Instanzen von Klassen vorgesehen, deshalb meine Frage, ob es tatsächlich falsch ist, die ein-instanzen-Klassen so zu vermeiden, wie ich es tue.

Vielen Dank für eure Antworten :-)
Für alle meine Codebeispiele gilt: Äußert bitte jegliche Art von Verbesserungsvorschlägen. Ich versuche immer meinen Stil zu verbessern und wenn man mir einfach sagt, was ich falsch machen, ist es um einiges einfacher, als wenn ich es mühselig selber herausfinden muss :-)
Benutzeravatar
__blackjack__
User
Beiträge: 13103
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@Septias: Warum magst Du das nicht? Es ist jedenfalls kein schlechter Stil nur ein Exemplar einer Klasse zu erzeugen.

Das was Du da machst würde ich als falsch ansehen. Das ist sehr überraschend und so nicht vorgesehen.

Selbst von Singletons möchte man eventuell mehr als ein Exemplar erstellen. Zum Beispiel bei Unit-Tests.
„All religions are the same: religion is basically guilt, with different holidays.” — Cathy Ladman
Benutzeravatar
noisefloor
User
Beiträge: 3856
Registriert: Mittwoch 17. Oktober 2007, 21:40
Wohnort: WW
Kontaktdaten:

Hallo,

was ist denn den das Ziel des ganzen? Klassen dienen ja (auch) dazu, mehr Struktur in den Code zu bringen und "Funktionalitäten" zusammen zu fassen.
Wenn dein Skript bei einem Durchlauf nur eine Instanz anlegt, ist das nichts schlimmes dran. Wenn du z.B. eine kleine GUI programmierst, die z.B. nur einen Button hat, dann nutzt du da ja auch nur eine Instanz der Button Klassen. Und in der Regel auch nur eine Instanz der Hauptklasse der GUI.

Gruß, noisefloor
Benutzeravatar
kbr
User
Beiträge: 1487
Registriert: Mittwoch 15. Oktober 2008, 09:27

@Septias: Es ist völlig ok, zur Strukturierung eine Klasse ohne Instanzen zu haben, oder nur eine Instanz einer Klasse.

Dein Beispiel hat durch die Kombination von @property mit @classmethod in der Tat ein überraschendes Verhalten. Eine Lösung wäre, einen expliziten Descriptor zu verwenden:

Code: Alles auswählen

class Layers:
    
    def __init__(self, value):
        self.value = value
    
    def __get__(self, obj, klass):
        return self.value
    
    def __set__(self, obj, value):
        self.value = value
        

class Config:
    noiseDistance = 30
    rotationX = 0
    rotationY = 0
    heightScale = 30
    layers = Layers(3)
        
    noiseCount = 50
    
    @classmethod
    def valSetter(cls, val):
        def f(x):
            cls.__dict__[val] = x
        return f

Wenn Du mit dem layer-value nichts spezielles anstellst, brauchst Du aber keinen Descriptor. Und 'valSetter' ist auch überflüssig, da diese Funktionalität auf Klassen durch den dot-Operator erreicht werden kann. So kannst Du Dein Beispiel komplett durch einen SimpleNamespace ersetzen:

Code: Alles auswählen

from types import SimpleNamespace

Config = SimpleNamespace(
    noise_distance=30,
    rotation_x=0,
    rotation_y=0,
    height_scale=30,
    layers=3,
    noise_count=50,
)
Sirius3
User
Beiträge: 17749
Registriert: Sonntag 21. Oktober 2012, 17:20

@kbr: ich finde, es ist nicht gut, KlassenDEFINITIONEN als Namensräume oder Datenspeicher zu mißbrauchen. Allein schon vom Schreibaufwand und der Unleserlichkeit, wenn man vor jede Methode @classmethod schreiben muß. Ein zusätzliches `config = Config()` erzeugt eine wirkliche Instanz und läßt sich genausogut, und an den Problemstellen sogar besser nutzen, als auf der KlassenDEFINITION zu arbeiten.
__deets__
User
Beiträge: 14539
Registriert: Mittwoch 14. Oktober 2015, 14:29

@Sirius3: full ack.
Benutzeravatar
kbr
User
Beiträge: 1487
Registriert: Mittwoch 15. Oktober 2008, 09:27

@Sirius3: bei 'KlassenDEFINITIONEN' hatte ich Klassen im Sinne, die *keine* Methoden implementieren. Denn dem TE ging es, so wie ich sein Beispiel verstanden hatte, um den Namensraum für Daten. Also genau der Anwendungsfall, für den SimpleNamespaces gedacht sind. Sein Beispiel zumindest lässt sich auf genau den Fall reduzieren. Das hatte ich vermutlich nicht explizit genug ausgedrückt. Ansonsten teile ich Deine Anmerkung.
Septias
User
Beiträge: 80
Registriert: Freitag 24. Juni 2016, 19:15

Ja, ich wollte durch die Verwendung einer Klasse nur den Namensraum für die Daten haben. Diesen brauche ich, weil ich einen Slider, um einen nummerischen Wert auszuwählen, so implementiert habe:

Code: Alles auswählen

class Selector:
    selectors = []
    targetedSelector = None
    def __init__(self, x, y, width, start, end, target):
        self.x = x
        self.y = y
        self.width = width
        self.height = 30
        self.start = start
        self.end = end
        self.selectors.append(self)
        self.sliderx = x + 5
        self.slidery = y + 5
        self.target = target
        self.range = end - start
        self.right = self.x + width
        self.bottom = self.y + self.height
        self.start = start
        
    def colision(self, point):
        if point[0] > self.x and point[0] < self.right and point[1] > self.y and point[1] < self.bottom:
            return self
    
    def set_state(self, x):
        if x > self.x and x < self.right:
            self.sliderx = x
            self.target(((self.sliderx - self.x) / self.width) * self.range + self.start)
        
    @classmethod
    def update(cls, point):
        if cls.targetedSelector:
            cls.targetedSelector.set_state(point[0])
            return True
    
    def _draw(self):
        fill(255, 255, 255)
        rect(self.x, self.y, self.width, self.height)
        line(self.x, self.y + self.height / 2, self.right, self.y + self.height / 2)
        ellipse(self.sliderx, self.y + self.height / 2, self.height*0.6, self.height*0.6)
        fill(0, 0, 0)
        text(((self.sliderx - self.x) / self.width) * self.range + self.start, self.sliderx, self.slidery)
        
    @classmethod
    def draw(cls):
        for selector in cls.selectors:
            selector._draw()
    
    @classmethod    
    def colisions(cls, point):
        for selector in cls.selectors:
            if selector.colision(point):
                cls.targetedSelector = selector
                return selector
    
    @classmethod
    def release(cls):
        cls.targetedSelector = None
Da es soweit ich weiß in Python keine Referenzen gibt, habe ich Gedacht, dass man den Slidern dann einfach eine Funktion mitgibt, mit dessen Aufrauf dann die zu verändernde Variable verändert wird. Um dann mit der Funktion die Variable verändern zu können musste ich sie aus dem globalen Namensraum in einen seperaten verschieben, weil man die Variablen ja sonst ohne das Verwenden von 'global' nicht verändern kann.
Deshalb auch die Funktion 'valSetter' in Config, um die beschriebene Funktion zu erstellen, und diesen Aufruf zu ermöglichen:

Code: Alles auswählen

Selector(5, 5, 300., 0, 200, Config.valSetter("heightScale"))
Ist das eine gute Lösung, oder gibt es einen besseren Weg die Selector zu implementieren ?

Die Property wollte ich haben, weil der Wert von manchen Variablen ein 'int' sein muss, und ich dann dachte, dass ich im setter der Property dann einfach die Umwandlung machen kann.
Habe es jetzt schlussendlich aber so gelößt:

Code: Alles auswählen

class Config:
    noiseDistance = 30
    rotationLevelX = 0
    rotationLevelY = 0
    heightScale = 30
    layers = 3
    scale = 1
    noiseCount = 50
    
    @classmethod
    def valSetter(cls, val, func=None):
        if func:
            def f(x):
                cls.__dict__[val] = func(x)
        else:
            def f(x):
                cls.__dict__[val] = x
        return f

Code: Alles auswählen

Selector(615, 5, 300., 1, 15, Config.valSetter("layers", int))
Ist zwar nicht wunderlich schön, aber in meinem Fall funktioniert es fürs Erste :-)
Für alle meine Codebeispiele gilt: Äußert bitte jegliche Art von Verbesserungsvorschlägen. Ich versuche immer meinen Stil zu verbessern und wenn man mir einfach sagt, was ich falsch machen, ist es um einiges einfacher, als wenn ich es mühselig selber herausfinden muss :-)
Sirius3
User
Beiträge: 17749
Registriert: Sonntag 21. Oktober 2012, 17:20

@Septias: Du scheinst gerne globale Variablen zu verwenden, was das Programm nicht gerade verständlich macht. Die globale Liste in `selectors` in `Selector` und `targetSelector` sollten in eine eigene Klasse Selectors wandern, wo auch die ganzen Klassenmethoden hin gehören.
Genauso ist die `Config`-Klasse eigentlich eher ein Wörterbuch, oder, falls Du Wert auf Attributzugriffe legst, irgendwas, das das bietet. Statt diesem magischen `valSetter` würde man dann auch einfach `setattr` oder beim Wörterbuch operator.setitem benutzen.

Funktionen sollten entweder nie etwas per `return` zurückgeben, oder immer, nicht mal im einen if-Pfad return benutzen im anderen nichts.

Wieso gibt es ein x/y und ein right/bottom? Warum kein left/top?

Code: Alles auswählen

from operator import setitem

class Selector:
    def __init__(self, x, y, width, start, end, on_change):
        self.x = x
        self.y = y
        self.width = width
        self.height = 30
        self.start = start
        self.end = end
        self.sliderx = x + 5
        self.slidery = y + 5
        self.on_change = on_change
        self.range = end - start
        self.right = self.x + width
        self.bottom = self.y + self.height
        self.start = start
        
    def collide(self, point):
        return self.x < point[0] < self.right and self.y < point[1] < self.bottom:
    
    def set_state(self, x):
        if self.x < x < self.right:
            self.sliderx = x
            self.on_change(((self.sliderx - self.x) / self.width) * self.range + self.start)
        
    def _draw(self):
        fill(255, 255, 255)
        rect(self.x, self.y, self.width, self.height)
        line(self.x, self.y + self.height / 2, self.right, self.y + self.height / 2)
        ellipse(self.sliderx, self.y + self.height / 2, self.height*0.6, self.height*0.6)
        fill(0, 0, 0)
        text(((self.sliderx - self.x) / self.width) * self.range + self.start, self.sliderx, self.slidery)
        
class Selectors:
    def __init__():
        self.selectors = []
        self.targeted_selector = None

    def add_selector(x, y, width, start, end, on_change)
        selector = Selector(x, y, width, start, end, on_change)
        self.selectors.append(selector)
        return selector

    def update(self, point):
        if self.targeted_selector:
            self.targeted_selector.set_state(point[0])
            return True
        return False
    
    def draw(self):
        for selector in self.selectors:
            selector._draw()
    
    def collide(self, point):
        for selector in self.selectors:
            if selector.colide(point):
                self.targeted_selector = selector
                return True
        return False
    
    def release(self):
        self.targeted_selector = None

config = dict(
    noiseDistance = 30,
    rotationLevelX = 0,
    rotationLevelY = 0,
    heightScale = 30,
    layers = 3,
    scale = 1,
    noiseCount = 50,
)

selectors = Selectors()
selectors.add_selector(615, 5, 300., 1, 15, lambda val: setitem(config, "layers", int(val)))
Benutzeravatar
__blackjack__
User
Beiträge: 13103
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@Septias: `selectors` und `targetedSelector` sollte es nicht geben. Das sind globale Variablen. Letztlich wiederholst Du hier das was Du bei `Config` machst, Du versuchst auf Kosten von komischem Code Exemplare zu ”sparen”. Dafür gibt es keinen Grund.

Einige von den `Selector`-Attributen würde ich als Properties implementieren, weil die sich aus anderen berechnen und es damit redundant ist die zu speichern.
„All religions are the same: religion is basically guilt, with different holidays.” — Cathy Ladman
Septias
User
Beiträge: 80
Registriert: Freitag 24. Juni 2016, 19:15

Vielen Dank für die vielen Vorschläge und Verbesserungen :)
Für alle meine Codebeispiele gilt: Äußert bitte jegliche Art von Verbesserungsvorschlägen. Ich versuche immer meinen Stil zu verbessern und wenn man mir einfach sagt, was ich falsch machen, ist es um einiges einfacher, als wenn ich es mühselig selber herausfinden muss :-)
Benutzeravatar
snafu
User
Beiträge: 6740
Registriert: Donnerstag 21. Februar 2008, 17:31
Wohnort: Gelsenkirchen

Übrigens, mit der gleichen Logik könnte man fragen ob es gut ist, Funktionen zu haben, die nur einmal aufgerufen werden. Auch hier ist die Antwort: Ja, weil es nicht nur um wiederholte Aufrufe geht, sondern auch um die bessere Strukturierung des Codes. Isolierte Einheiten sind besser testbar und überschaubarer. Damit steuern sie zur Verbesserung der Codequalität bei.
Antworten