Terme zusammenfassen

[starki]

Hey Leute,

ich schreibe gerade ein kleines Programm, mit dem ich Terme darstellen kann. Mit Termen meine ich in der Mathematik sowas wie: 4t + 5t - 9a usw.

Dafür habe ich zwei Klassen geschrieben:

Code: Alles auswählen

class Term(object):
	def __init__(self, zahl, variable):
		self.Zahl = zahl
		self.Variable = variable

und

Code: Alles auswählen

import term
import math

class Formel(object):
	def __init__(self, terme):
		self.Terme = terme
	
	def zusammenfassen(self):
		if len(self.Terme) <= 1:
			return self.Terme
		
		terme = []
	
		while len(self.Terme) != 0:
			t = self.Terme[0]
			del self.Terme[0]
			for i in range(len(self.Terme)):
				try:
					if t.Variable == self.Terme[i].Variable:
						t.Zahl = t.Zahl + self.Terme[i].Zahl
						del self.Terme[i]
				except Exception as ex:
					pass
			terme.append(t)
		self.Terme = terme
		return terme
	
	#erstellt einen string aus der formel
	def get_formel(self):
		res = ""
		for i in range(len(self.Terme)):
			if self.Terme[i].Zahl > 0:
				res = res + " + " + str(self.Terme[i].Zahl) + self.Terme[i].Variable
			elif self.Terme[i].Zahl == 0:
				pass
			else:
				res = res + " - " + str(abs(self.Terme[i].Zahl)) + self.Terme[i].Variable
		return res
	
	#schreibt die formel auf die konsole
	def f_print(self):
		print(self.get_formel())

Probleme bereitet mir die Funktion zusammenfassen aus der Klasse Formel. Man hat ja beispielsweise eine Formel wie oben beschrieben: 4t + 5t - 9a, diese zusammegefasst, ergibt: 9t - 9a
Jedoch gibt es manchmal Probleme mit dieser Funktion.

Beispielsweise bei einer Formel wie dieser:

- 3h - 13f + 7f + 7i + 4f + 14f + 12h + 10i

kommt folgendes heraus:

+ 9h - 2f + 17i + 14f

Was ist falsch an meiner Funktion ?

[Sirius3]

die gröbsten Fehler:
Du änderst self.Terme innerhalb Deiner for-Schleife.
Welche Exception soll denn da abgefangen werden?

Das Konstrukt:

Code: Alles auswählen

for i in range(len(liste)):
    tu was mit liste[i]

ist generell schlecht und sollte bei Dir durch

Code: Alles auswählen

for term in self.Terme:
  ...

ersetzt werden, dann läufst Du auch nicht in den Fehler, die Liste
innerhalb von for ändern zu wollen.

[starki]

die gröbsten Fehler:
Du änderst self.Terme innerhalb Deiner for-Schleife.
Welche Exception soll denn da abgefangen werden?

OutOfIndex-Fehler. Also wenn ja ein Element aus der Reihe gelöscht wird, kann die Schleife ja nicht mehr auf die letzte Position zugreifen, da ja das letzte Element auf der vorletzten Position ist...

Wie kann ich die Funktion besser schreiben? Ich hatte zwei drei Varianten bisher, aber alle haben leider nicht so funktioniert, wie sie sollten und auf neue Ideen bin ich bisher nicht gekommen ...

[Sirius3]

Ich denke, collections.Counter tut genau das was Du willst.

[starki]

Ich denke, collections.Counter tut genau das was Du willst.

Hey danke, die kannte ich noch gar nicht. Meine Funktion sieht jetzt so aus und scheint auch ganz gut zu funktionieren ... :

Code: Alles auswählen

	def zusammenfassen(self):
		if len(self.Terme) <= 1:
			return self.Terme
			
		cnt = collections.Counter()
		for t in self.Terme:
			cnt[t.Variable] += t.Zahl
		
		terme = []
		for l in list(cnt):
			t = term.Term(cnt[l], l)
			terme.append(t)
		
		self.Terme = terme

[BlackJack]

@starki: Ich würde das erste ``if`` ja einfach weglassen. Oder die Bedingung negieren und den Rest in den Block verschieben.

`collections.Counter` kann man zwar verwenden, aber ich glaube ich würde `collections.defaultdict` benutzen. Denn vom `Counter` wird hier ja nichts verwendet was der zusätzlich bietet.

Die Namen sind teilweise schlecht gewählt. Man sollte die Sachen einfach ausschreiben und nicht kryptisch abkürzen. Besonders schlecht ist l weil man in vielen Schriftarten l und 1 nicht wirklich gut voneinander unterscheiden kann.

Der `list()`-Aufruf ist unnötig und wenn man Schlüssel *und* Wert innerhalb der Schleife benötigt, sollte man auch direkt über beides iterieren. Die Schleife könnte man auch prima zu einer „list comprehension” zusammenfassen.

Bei `Formel.get_formel()` passt der Name nicht so ganz zum Inhalt. Bei dem Namen würde man irgendwie erwarten, dass ein `Formel`-Objekt zurück gegeben wird. Es wird aber eine Zeichenkette geliefert. Um ein Objekt in eine Zeichenkette für Benutzer umzuwandeln ist die `__str__()`-Methode gedacht. Dann braucht man auch keine spezielle `print()`-Methode sondern kann gleich die `print()`-Funktion verwenden. Was soll eigentlich das `f_` bei der Methode bedeuten?

Für den postitiven und den negativen Fall wird eine Menge Quelltext wiederholt, obwohl die sich wenn man auch beim positiven Fall `abs()` anwendet nur an *einer* Stelle unterscheiden. Die 0 könnte man eventuell hier gar nicht besonders behandeln und den Fall mit in die Methode zum Zusammenfassen verlagern.

Warum ist `Term` bei Dir in einem eigenen Modul? Python ist nicht Java — man steckt nicht jede Klasse in eine eigene Datei.

Komplett ungetestet:

Code: Alles auswählen

from collections import defaultdict


class Term(object):
    def __init__(self, factor, variable):
        self.factor = factor
        self.variable = variable
    
    def __cmp__(self, other):
        return cmp((self.variable, self.factor), (other.variable, other.factor))
    
    def __str__(self):
        return '{0}{1}'.format(
            (self.factor if self.factor != 1 else ''), self.variable
        )


class Formula(object):
    
    def __init__(self, terms):
        self.terms = terms
    
    def __len__(self):
        return len(self.terms)
    
    def __iter__(self):
        return iter(self.terms)
    
    def __str__(self):
        result = ''.join(
            ' {0} {1}{2}'.format(
                ('-' if t.factor < 0 else '+'),
                (abs(t.factor) if t.factor != 1 else ''),
                t.variable
            )
            for t in self)
        return result[1 if result[1] == '-' else 2:]
    
    def collect(self):
        variable2factor = defaultdict(int)  # Oder `float`.
        for term in self:
            variable2factor[term.variable] += term.factor
        self.terms = sorted(
            Term(f, v) for f, v in variable2factor.iteritems() if f != 0
        )