Ocena projektu - kalkulator w Pythonie (tkinter)

Question

Witam. Od jakiegoś czasu piszę sobie kalkulator w Pythonie. Posiada on już dość dużo funkcji i zbliża się do poziomu funkcjonalności który mnie zadowala i na którym zamierzam zakończyć jego rozwój i skupić się na innych projektach. W związku z tym chciałbym prosić o ocenę mojego projektu. Kod znajduje się na GitHubie, instalator można pobrać w zakładce releases.

Chętnie przyjmę wszelkie rady, opinie dotyczące kodu jak i samego kalkulatora oraz pomysły na rozwój czy też informacje o znalezionych błędach. Może nawet komuś się spodoba i zacznie używać go zamiast kalkulatora z Windowsa 

Comments

A gdzie testy?

Answer 1

Hmm, po kolei...

 - gdzie requirements.txt (lub odpowiedniki typu pyproject.toml)

- czemu customowa biblioteka do tłumaczeń zamiast np wbudowanego gettext?

- klasa `Tools`, nie ma sensu istnienia, a tym bardziej to dziwne dziedziczenie. To powinny być zwykłe wolne funkcje.

- if, elif etc nie potrzebują nawiasów

- zamiast `def size()`, używaj standardowe sygnatury typu __len__. (osobiście bym po prostu użył listę zamiast klasy wrapującej, ale to inny temat)

	def isNumber(self, str):
		if(str.isdigit()):
			return 1
		else:
			try:
				float(str)
				return 2
			except ValueError:
				return 0

1, 2? To nie C, możesz po prostu zwrócić True.

			if(re.match(r'^[01]+$', number)):
				number = int(number,2)
			else:
				messagebox.showerror(_("error"), _("err2"))
				return

Regex raczej niepotrzebny, wystarczy wywołać int(number, 2) i złapać wyjątek.

To tak bardzo na szybko z kilku pierwszych linii kodu.

Comments

Dzięki za wskazówki, postaram się wprowadzić zmiany.

Muszę wreszcie zadbać aby inni mogli łatwo odpalić kod i nie chodzi tylko o requirements.txt.

Customowa biblioteka do tłumaczeń dlatego że gettext wydawał mi się trochę niepotrzebnie skomplikowany jeśli chodzi o strukturę katalogów i tworzenie tłumaczeń.

Klasa Tools nie ma sensu ale nie wiedziałem jak sobie z tym poradzić, nie wiem dlaczego użycie tego jako funkcji poza jakąkolwiek klasą wydawało mi się niewłaściwe.

Wiem, że nawiasy w if są niepotrzebne, po prostu takie dziwne przyzwyczajenie, że akurat w if je daję choć nie są wymagane.

Zwracanie 1 lub 2 w funkcji isNumber daje możliwość sprawdzenia dodatkowo czy jest to int czy float. Pewnie lepszym rozwiązaniem byłoby podzielenie tego na isInteger() i isFloat() zwracające True/False.

Answer 2

1. dziwne ustawienia formatowania kodu, bardzo duże wcięcia, długie linijki

2. w kilku miejscach używasz w pętli listy [0, 1, 2, 3, 4, 5], bardziej eleganckie byłoby range(6)

3. podział na pliki. każda klasa do osobnego pliku, łatwiej się potem czyta kod

4. brak testów

Comments

Dzięki za wskazówki, postaram się wprowadzić zmiany.

Answer 3

1. Brak testów. W przyszłości ktoś kto będzie chciał (albo z konieczności będzie zmuszony) utrzymywać aplikacje, zastosuje mnożnik do wyceny usługi.

2. Brak należytej modularyzacji. Nawet na poziomie warstw (np. warstwa logiki kalkulatora i jego prezentacji).

3. Pierwsze czego szukałem to:

if __name__ == '__main__':

.. nie ma, już wiem że będzie ciekawie....

4. 1 klasa Stack... co ma ten stos czego nie ma zwykła... lista?

5. Przy klasach budujących GUI, praktykuje się dziedziczenie z Frame lub innego okna/widgetu. Wtedy dostęp do elementów nie jest rozbudowany przez self.coś.gdzieś.komujś. Poza tym wygląd warto wyprowadzić do oddzielnego kontenera a nie "dziergać tu i ówdzie w kodzie".

5. Narzędzie pep8 (tu masz online: http://pep8online.com/ ) zgłasza zastrzeżenia.

6. Ja bym ostrzegał że konwersja waluty wymaga dostępu do sieci.

7. Połączenie mnożników i jednostek, proponuję zrobić w jednym kontenerze a nie rozbijać na osobne. Łatwiej to utrzymać i znaleźć błąd.

8. Jawne złamanie DRY. Wszystkie konwertery mają taki sam kod a jedynie różnią się zestawem jednostek.

9. ~1200 linii... naprawdę? Miejże litość...

10. Brak łatwej instalacji. Może zipapp, może jeszcze inaczej.. ale nie tak ..

 

Comments

Dzięki za wskazówki, postaram się wprowadzić zmiany.