search

wybory2014 / Kalkulator1

363 stars 105 forks source link

Refactor #2

Open robin92 opened 9 years ago

robin92 commented 9 years ago

Ktoś chętny do refactoru paru klas?

rbtbar commented 9 years ago

całość do zaorania.

lsolniczek commented 9 years ago

A może przepisać to na wersję webową np w Railsach?

McOffsky commented 9 years ago

Zorać i postawić od nowa w wersji webowej. Ciekaw jestem jak wygląda backend.

dzaba commented 9 years ago

Wystarczy zrefactorowac jedna klase :) ProtocolForm.cs tylko 15k linii kodu, chyba mozna powolac sie na konwencje genewska aby tego nie ruszac.

hardc0de commented 9 years ago

@dzaba Raczej na klauzulę sumienia ;)

mordka commented 9 years ago

Panowie nie ruszajcie kodu bo zlamiecie ustawe o zamowieniach publicznych ;)

hardc0de commented 9 years ago

Moim zdaniem całe przedsięwzięcie powinno być od początku do końca jawne, z publicznie dostępnym kodem źródłowym. Wybory to sprawa społeczna, kasa na projekt idzie z kieszeni podatników, więc możliwość sprawdzenia przez obywatela na co idą jego pieniądze jest wręcz obowiązkiem naszego państwa.

rr- commented 9 years ago

A może przepisać to na wersję webową np w Railsach?

http://www.joelonsoftware.com/articles/fog0000000069.html

Ciekawe, co autor by powiedział widząc tę aplikację.

hardc0de commented 9 years ago

@rr: Z powyższego artykułu: "One project I worked on actually had a data type called a FuckedString" Jeżeli to jeden z gorszych problemów, na jakie trafił autor to sądzę, że załamałby się po zobaczeniu tego ;)

rbtbar commented 9 years ago

@dzaba Chętnie pomogę, ale nie mam tak szerokiego ekranu.

1

bkalota commented 9 years ago

Podejrzewam, że bez problemu znalazłoby się paru chętnych, którzy wystrugaliby to za free, po godzinach, ewentualnie w zamian za wpis w CV. Ale patrząc na relację mediów, sądzę, że infrastruktura + wiedza domenowa byłaby dla zamawiającego poważnym problemem. I jeszcze do tego web formsy...

rr- commented 9 years ago

@xxxbartasxxx podczas gdy na pewno by się tacy ludzie znaleźli, nie sądzę, żeby stworzyli oni dużo lepszy produkt. Kluczowym problemem jest tutaj czas, którego spora część musiała zostać przeznaczona na np. szkolenie ludzi i inne "papierkowe" problemy. Oczywiście, można powiedzieć, że dobrze zrobiona aplikacja nie powinna potrzebować szkoleń ani instrukcji, jednak jeśli chodzi o państwowe projekty, pewne procedury są wymagane "z urzędu" :unamused:

Czas to także dobry powód dla którego zastosowanie modelu konkursu na najlepiej działającą aplikację również byłoby nierealne - ocena poszczególnych rozwiązań trwałaby za długo.

Myślę że podobny tok rozumowania przyjęły te firmy, które odmówiły realizacji, i w efekcie na rynku zostali tylko ludzie zdesperowani i/lub niedoświadczeni... czego potwierdzenie widzimy w tym kodzie.

pklebba commented 9 years ago

Tylko w jakim celu skoro i tak nikt w przyszłości na 99% z tego nie skorzysta? Hobbystycznie?

skrzyp commented 9 years ago

@Siprah "W imię zasad" ;) btw. Lodówkę otworzy człowiek, a tu znajomi.

bkalota commented 9 years ago

@rr- Tak naprawdę ten kod nie jest problemem. To tylko wynik absurdów, które doprowadziły do jego powstania.

@Siprah Pisanie tego od nowa, kiedy mleko się wylało, nie ma oczywiście żadnego sensu.

skrzyp commented 9 years ago

Ale z drugiej strony, można by spróbować wspólnymi siłami i pokazać, że "internety zrobią lepiej" (bo zrobią, wierzę że zrobią :>). Ale ja bym jednak preferował, żeby to zostało w takiej formie jakiej jest, czyli .NET/C# i dodawać PRy do repo, albo nowego brancha zrobić ;>

dzaba commented 9 years ago

@xxxbartasxxx :

"Tak naprawdę ten kod nie jest problem. To tylko wynik absurdów, które doprowadziły do jego ?> powstania."

Wyjąłeś mi to z ust, jeżeli zlecamy zadanie frontendowcowi php to takich efektów możemy oczekiwać. Ale z tego co czytam, oglądam to problemem jest chyba sam serwer niż aplikacja kliencka,

Na filmiku widać, że jest to aplikacja webowa: http://www.tvn24.pl/komunikat-brak-uprawien-informatyk-komisji-z-sopotu-walczyl-z-systemem-problemy-w-innych-miastach,489656,s.html

Kicer86 commented 9 years ago

To co mnie zastanawia, to to, dlaczego nie wykorzystano rozwiązań z uprzednich wyborów. Miałem styczność z wersją, z bodajże, 2006 roku i nie było problemów. Nie wystarczyło wymienić listy nazwisk?

rr- commented 9 years ago

Może nazwiska były zaszyte w kodzie?

protip: żart

Kicer86 commented 9 years ago

Nawet jeśli, to taniej i bezpieczniej byłoby wydać nowszą wersję działające i sprawdzonej już apki, zamiast tworzyć nową. Zupełnie nie rozumiem tego podejścia.

hardc0de commented 9 years ago

"Dzieło" które właśnie komentujemy trafiło nawet na Reddita: https://www.reddit.com/r/programming/comments/2ml27h/source_code_of_polish_electoral_calculator_big/

sarxos commented 9 years ago

Pomijając szereg błędów w samym kodzie źródłowym i absurdów przy tworzeniu tego projektu, bez sensu moim zdaniem było tworzenie aplikacji w formie pliku wykonywalnego. Nie łatwiej było zrobic web aplikację? Panie i panowie z firmy krzak na pewno lepiej by sobie z tym poradzili, a interwencja na poziomie kodu źródłowego w momencie fakapu była by łatwiejsza, no i po naprawieniu błądu na serwerze nie trzeba pałowac się z ponowną dystrybucją plików exe do wszystkich uczestników procesu wyborczego.

hardc0de commented 9 years ago

@sarxos:

Nie łatwiej było zrobic web aplikację?

Na pewno efekt końcowy byłby lepszy, ale najpierw trzeba umieć te internety robić ;) Wyklikać webówkę już nie tak prosto jak aplikację WinFormsową.

pklebba commented 9 years ago

@hardc0de To kwestia UI/UX, a nie tego, że jest to jest webaplikacja.

sarxos commented 9 years ago

@hardc0de

najpierw trzeba umieć te internety robić ;)

Prawda :)

A tak by the way. W czasie testów (kiedy oczywiście też był fakap) binarki kalkulatora były w wersji 2.0.0.44, natomiast w chwili gdy to piszę aktualną wersją jest 2.0.0.102. Czyli jak mniemam, zrobili 58 releasów, a nie byli w stanie naprawic błędów wykrytych w czasie testów :astonished: Zastanawia mnie co oni w tym czasie kodowali?

Ale nie dziwię sie wcale, że wyszło jak wyszło, skoro rutyną w tym kodzie jest połykanie wyjątków:

catch (XmlException)
{
}
catch (System.NullReferenceException)
{
}
hardc0de commented 9 years ago

@Siprah UI/UX to jedno, ale pisząc webówkę poza samym .NET/C# nasza biedna Agnieszka musiałaby dodatkowo orientować się w: HTML/CSS, JavaScripcie/JQuery, AJAXie, pewnie do tego jakiś Bootstrap żeby było szybko i jakoś wygladało. Nie wiem jak to wyląda u autorki w praktyce, ale sam dobrze wiem, że przy braku doświadczenia żonglowanie kilkoma technologiami jednocześnie potrafi być naprawdę ciężkie. Strach pomyśleć ile wtedy zajęłoby napisanie tego.

McOffsky commented 9 years ago

Ona podpisuje się jako programistka PHP, więc css i html raczej nie były by dla niej problemem. Pewnie było tak że janusze-prezesi złapali przetarg i huzzia, każdy komu słowo "kod" kojarzy się z czymś innym niż enigma został zagoniony do C#. Ale z drugiej strony, jako osoba zaczynająca w php i obecnie bawiąca się w .net i javie nie rozumiem jak doświadczony programista jakiegokolwiek języka jest w stanie wytworzyć taki kod.

sarxos commented 9 years ago

jak doświadczony programista jakiegokolwiek języka jest w stanie wytworzyć taki kod

@McOffsky, student potrafi ;)

hardc0de commented 9 years ago

@McOffsky:

[...] raczej [...]

Z tym jak wiadomo bywa różnie.

doświadczony programista jakiegokolwiek języka

A gdzie masz napisane, że autor(ka) to doświadczony programista? ;)

McOffsky commented 9 years ago

@sarxos Chapnij pół miliona za przetarg na appkę do demokratycznych wyborów, napisz studentami za jakieś 20tys w sumie za 3 miesiące, nie zatrudnij nikogo do audytu kodu appki :V sukces po polsku.

@hardc0de Staram się być optymistą ;P

tporadowski commented 9 years ago

Właśnie przejrzałem SIWZ dla tego przetargu - i nie ma tam wymogu, by była to aplikacja "desktopowa". Ale jest światełko w tunelu - jest nowe ogłoszenie przetargu (z 20 paź) na rozbudowę i wykonanie modułów... do tegoż stworka, ktoś chętny? ;)

hardc0de commented 9 years ago

napisz studentami

Piękne określenie :laughing: Myślę, że prezes Comarchu byłby dumny :wink:

tporadowski commented 9 years ago

@hardc0de @McOffsky SDP czyli Student-Driven Programming?

rpodwika commented 9 years ago
randomness 
  byte[] salt = new byte[10]
      {
        (byte) 1,
        (byte) 2,
        (byte) 3,
        (byte) 4,
        (byte) 5,
        (byte) 6,
        (byte) 7,
        (byte) 8,
        (byte) 9,
        (byte) 10
      };
hardc0de commented 9 years ago

@tporadowski: A nie przypadkiem SDD, czyli Student-Driven Development? Mogę się mylić :wink:

dzaba commented 9 years ago

tutaj raczej zadziałało WHISKY = "Why The Hell Isn't Somebody Koding Yet".

sarxos commented 9 years ago

Ciekawe komentarze można znaleźć pod informacjami o testach:

JJgmina: W mojej gminie częściowo się udało, operator główny w końcu pod wieczór dostał wyniki, ale opóźnienie było ogromne. Byłam uparta i nie zliczyłabym kliknięć na wyślij, a i tak gdy po milionie kliknięć i 3 godzinach spędzonych bezowocnie przy komputerze pojawił się komunikat, że nie można wysłać protokołu po raz 2, nie byłam pewna, czy rzeczywiście poszło. Pomimo wysłania napis przy protokole nie pojawił się napis wysłane. Mam tylko nadzieję, że podczas wyborów nie będzie powtórki z testu...

Zastanawiam się, czy w czasie tego klikania, protokół nie był wysyłany przypadkiem ponownie, bo w kodzie nie widzę, aby gdziekolwiek ten button był blokowany ;P

Ciekawy jest też if:

if ((
  !this.errorValue.Visible && !this.errorHour.Visible && !this.errorOBW.Visible && 
  !this.errorCurrentLwyb.Visible) || (
  !this.errorValue.Visible && !this.errorHour.Visible && !this.errorOBW.Visible && 
  this.errorCurrentLwyb.Visible && 
  this.errorCurrentLwyb.Text == "Liczba wyborców uprawnionych do głosowania jest mniejsza od 110% i większa od 90% szacowanej liczby wyborców (" + 
  (this.obwodList.SelectedItem as AttendanceOBWItem).getLwyb().ToString() + ")."))
{

disgusted-mother-of-god

tporadowski commented 9 years ago

@hardc0de Development to inny stopień wtajemniczenia :wink:

sarxos commented 9 years ago

Oprócz oczywistego deficytu jakości, firma krzak jest troszku na bakier ze specyfikacją zamówienia z PKW:

14 Wydruki oraz formularze i komunikaty ekranowe muszą korzystać ze słowników nazw własnych, zdefiniowanych dla jednostek terytorialnych, okręgów wyborczych, wybieranych instytucji i organów wyborczych itp. (np. nazwy własne w poszczególnych przypadkach gramatycznych). Zapisy słownikowe mogą podlegać modyfikacji (z wersjonowaniem według przedziału czasowego, w jakim obowiązują).

A w kodzie wszystkie nazwy, komunikaty, etc zahardcodowane jak byk.

hardc0de commented 9 years ago

Do pełni absurdu brakuje mi tutaj tylko użycia bogosorta :D

keraxel commented 9 years ago

@wybory2014 Przyjmujecie pull requesty?

rafaltra commented 9 years ago

@Keraxel Chyba kondolencje

TheMolkaPL commented 9 years ago

Już jesteśmy 5 w miesięcznym rankingu GitHuba C#! Ranking

riklaunim commented 9 years ago

W sumie dobrze by wyszło gdyby z tego przypadku powstał narodowe wyzwanie implementacji kalkulatora w $ulubionym języku z code review, wskazówkami itd. jak to zrobić porządnie od reszty "społeczności" skupionej wokół kalkulatora - czyli nauka programowania na "wesoło", no i kolejne wybory niebawem :) Wielu początkujących nie ma szansy na naukę z pomocną ręką doświadczonych programistów.

lauriys commented 9 years ago

za logicznie mówisz; nie ten kraj

michal-franc commented 9 years ago

Estonia ma system dostepny publicznie! Dlatego moze tez udaloby sie zmobilizowac i napisac cos takiego ?

lauriys commented 9 years ago

@michal-franc przeczytaj komentarz nad twoim :D

kminek commented 9 years ago

mysle, ze lektura niektorych obowiazujacych przepisow prawnych dawalaby mniej wiecej podobny efekt, co lektura tego kodu...

wrzasa commented 9 years ago

Super... wątek wypływa już wszędzie. Jak tak dalej pójdzie za parę dni ,,polski programista'' będzie na całym świecie synonimem kompletnego idioty. Nie wiadomo na jak długo. Pozdrawiam moich kompetentnych kolegów po fachu...

siefca commented 9 years ago

Wiele projektów robionych na szybko tak wygląda i to jeszcze nie przesądza o ich klęsce. Gdyby było zarządzanie jakością (realne testy UAT, automatyczne testy jednostkowe, testy wydajnościowe, code reviews) w trakcie, to by część błędów wyłapano znacznie wcześniej i nawet byśmy się nie dowiedzieli o tym, że coś było nie tak, a w międzyczasie ktoś by usiadł i przepisywał to cudo przez parę miesięcy na spokojnie, żeby w kolejnym przetargu było sprzedane jako ulepszona wersja (np. do obsługi innych wyborów).

Widziałem programy naprawdę renomowanych firm i na początku wiele podprogramów było podobnej jakości jak tutaj metody – spaghetti code; pobieranie połowy bazy danych i filtrowanie tego po stronie klienta; czytanie czegoś w kółko, aż przestanie rzucać wyjątkiem; używanie ORM-ów na wyrost i dzierganie w nich zapytań specyficznych dla konkretnej bazy (bez używania procedur czy widoków); wstawianie haseł na sztywno czy kluczy licencyjnych w kod; wstawianie URL-i wiodących do serwera producenta, żeby coś dociągać; pozostałości po testach (jakieś debugi plujące w zawartość wyświetlaną klientowi czy do logów); zmienne w stylu dupa1 czy dupa2 (do dupa50… i weź się domyśl, która z dup za co odpowiada). No i parę miesięcy po wdrożeniu ktoś siadał i to poprawiał, a teraz tych błędów już tam nie ma, a i aplikacje przerobione, aby ładnie wyglądały. Sukces marketingowy był, wcześniej było niż u innych, to co się mogło rzucać w oczy lub nie działać zostało poprawione, żeby dawało radę. No ale trzeba prace nad jakością – trzeba siać siać siać te scenariusze! i prowadzić system śledzenia błędów.

Podsumowując: to by się dało uratować, gdyby tam zarządzanie ryzykiem projektowym ktoś traktował poważnie.