Lepiej pisać kod czytelny czy optymalny?

Witam, zastanawiam się właśnie jak pisać (panie premierze)?

Prosty przykład

function zrobCos(i) {
    var newValue;
    switch (i) {
        case 1: 
            newValue = 250; 
            break;
        case 2:
            newValue = 850;
            break;
    }
    $("#cos").css({margin: newValue});
}

Oczywiście można by to zapisać również tak

function zrobCos(i) {
    if (i == 1) newValue = 250;
      else newValue = 850;

    $("#cos").css({margin: newValue});
}

albo tak

function zrobCos(i) {
    $("#cos").css({margin: ( (i==1) ? 250 : 850 )});
}

I teraz mam dylemat, najkrótszy jest ten ostatni, ale patrząc na ten pierwszy kod od razu wiadomo o co chodzi, tzn jest najbardziej oczywisty, a różnice w switch i con?true:false pod względem czasu wykonania nie są jakieś duże. Jak uważacie, jak "się powinno" pisać?

krótszy kod != optymalny. Prawie zawsze da się napisać czytelnie optymalny kod.

Zredaguje pytanie :D bo nie o to mi chodziło.

Lepiej pisać czytelny kod, czy krótki kod?

zawsze czytelny, aczkolwiek czy przykład 1 jest czytelniejszy niż 2 to bym się kłócił

Krotki i czytelny.

Dla mnie ostatnia, najkrótsza wersja jest najczytelniejsza (akurat w tym przypadku), aczkolwiek zmienne nazwałbym bardziej opisowo.

nie wiem co to za język, ale ten pierwszy kod chyba robi trochę coś innego niż te 2 poniżej. np. dla i=3. Dałeś taki przykład, że nawet ta 3 wersja jest czytelna :P

akurat trzeci kod jest dla mnie najczytelniejszy - po spojrzeniu od razu wiedziałem o co chodzi, pierwsze dwa w ogóle pominąłem wzrokiem bo stwierdziłem że wymagają większej analizy a dopiero trzeci kod mnie wyprowadził z błędu

najlepiej pisać czytelny kod i maksymalnie dużo kodu wydzielać do osobnych funkcji - przyjmuje się że jedna funkcja/metoda powinna mieć max 10 linii kodu (co nie oznacza oczywiście że masz pisać niezrozumiały kod w jednej linii z kilkoma średnikami ;))
optymalizacją i "zinline'owaniem" niech zajmie się kompilator

w przypadku javascript akurat nie masz kompilatora jako takiego, ale silniki js w locie dużo optymalizują i masz narzędzia typu Closure compiler, który przykładowo kod:

 if (i == 1) var newValue = 250;
      else newValue = 850;

    $("#cos").css({margin: newValue});

"kompiluje" do postaci:

var newValue=1==i?250:850;$("#cos").css({margin:newValue});

i niech ta pierwsza forma właśnie będzie dla programisty, a druga dla przeglądarki

optymalizacją generalnie nie przejmuj się dopóki nie jest wymagana - dopiero jak coś działa za wolno to postaraj się to przyspieszyć
zazwyczaj próbuje się optymalizować nie to co trzeba

Dopiero co byl topic na podobny temat.
Kod ma byc czytelny dla potomnych programistow. kompilator i tak go zoptymalizuje po swojemu.

Takie rzeczy mają niewiele wspólnego z optymalizacją. Należy pisać przede wszystkim czytelnie, natomiast jeśli musimy optymalizować, to należy optymalizować kod, którego kompilator nie jest w stanie zoptymalizować. Kompilatory są niezłe w niskopoziomowych, lokalnych optymalizacjach np. rozwijaniu pętli lub przydzielaniu odpowiednich rejestrów na zmienne, ale już np. kompilator nie zastąpi Ci listy jednokierunkowej wektorem ani sortowania bąbelkowego quicksortem.

W Corpo na review kodu dostalbys issue - uzywanie magic numbers. Skad potem ktos ma wiedziec co poszczegolne numerki znacza?

Zasady optymalizacji Jacksona:

1. Nie optymalizuj
2. (dla ekspertów) jeszcze nie optymalizuj.

Wszystkie trzy przypadki podane przez ciebie nie wnoszą nic do optymalizacji. Co więcej można by to zapisać jeszcze ciekawiej i moim zdaniem jeszcze czytelniej:

 $("#cos").css({margin: newValuesMap[i]});

gdzie w mapie trzymasz wartości i odpowiadające im wartości i. Zapewne można by też wtedy taką mapę odpowiednio skomentować i w razie czego łatwo zrekonfigurować.

Co zaś tyczy się ogólnych zasad to warto pisać kod czytelny. Jak już okaże się, że jakiś fragment rzeczywiście wymaga optymalizacji to będzie przynajmniej prosto zrozumieć co kod robi i dzięki temu odpowiednio go zrefaktoryzować. Kod czytelny to nie tylko kod, który jest łatwo zrozumieć. Warto poświęcić czas i napisać testy, któe świetnie sprawdzają się do tłumaczenia co dany kod rzeczywiście robi.

W pierwszej kolejności kod w miarę czytelny, najlepiej żeby druga osoba nie musiała pytać Mnie "co to jest i co to robi", później jak już się okaże że jest wolne to nad tym myśleć. Czasami fajnie jest napisać kod i testy (nie mówię już o sytuacji gdzie testy są pierwsze co jednak jest trudne) i dzięki temu można poprawiać i testy i kod (w testach też są często błędy a i sam kod i to jak działa, i często patrzenie w code coverage pozwala pokazać czego w testach jeszcze nie ma - w sensie jakichś przypadków brzegowych). Po tym będą już napisane testy które nie powinny się zmieniać a więc mam "dupochron" do robienia refactoringu w przyszłosci kiedy będą go wymagali lub poprawiania czytelności kodu kiedy mam łatwy sposób na udowodnienie że to faktycznie działa, a nie że od teraz mam piękny kod który działa w 80% przypadków.

Przy okazji rozważ przypadek sortowania w mitycznej sytuacji kiedy samemu się te algo pisze (to tylko jako zobrazowanie, popatrzeć na pseudokody kolejnych algorytmów):
1 implementacja: bąbelkowe, da się szybko napisać, przetestować, udowodnić bez specjalnego myślenia 10 ludzi. (http://en.wikipedia.org/wiki/Bubble_sort)
2. kiedy okazuje się że 1. działa wolno..powstaje coś na zasadzie kodu w stylu HeapSort (http://en.wikipedia.org/wiki/Heapsort) - kodu w cholere i trzeba się ostro zmóżdżać o co w tym chodzi. Ale działa bardzo szybko i na ogół wymaga jakiegoś większego pomysłu, tutaj bazowania na kopcu a nie prostego porównania dwóch sąsiadów.
3. Kiedy już mamy działający i szybki kod ale nie da się go modyfikować z powodu jego zawiłości..powstaje rozwiązanie w stylu QuickSort (http://en.wikipedia.org/wiki/Quicksort) zwięzłe,szybkie,czytelne - tylko dlatego że wiemy po co to robimy.

i taka mała dygresja z mojej strony, czasami zdarza się że w kodzie powiedzmy czysto jawizmowym ktoś zaczyna pisac piękne kawałki z naleciałościami z innych jęzeyków (np Scala) i próbuje tworzyć konstrukty które troszkę tę scalę próbują przypominać (w "czystej" javie). Dla mnie to katorga jak mam przełączać co chwilę sposób myślenia bo ktoś się wyłamuje od ogólnego stylu kodowania w zespole bo część jest tak a część tak. (Nie żebym coś miał do innych języków - po prostu chodzi mi o mieszanie, jak cały zespół się zgadza to super, ale nie kawałki)

Dla mnie wszystkie wersje są złe. Powinieneś dodać odpowiednią klasę w zależności od warunku, a nie ustawiać na sztywno styl ;)

Tylko pamiętaj:

"premature optimization is the root of all evil"

Generalnie kod powinien mieć 3 wartości: komunikatywność, prostota, elastyczność. W tym przypadku jak widzę, wszyscy zrozumieli co kod robi, bez większych problemów, więc jakoś specjalnie komunikatywnie go usprawniać nie trzeba, chociaż nazwy zmiennych i funkcji na pewno powinny być bardziej wymowne. Inna sprawa tyczy się elastyczności kodu i w tym wypadku pierwsza opcja jest najbardziej elastyczna, chociaż oczywiście można to napisać jeszcze bardziej elastycznie, jednak elastyczność nie powinna być kosztem komunikatywności, chyba, że jesteśmy pewni, że ta się później przyda. Trzecią wartością jest "prostota", czyli brak złożoności ;) Prostota często jest mylona z komunikatywnością, jednak są to dwa różne aspekty, które przeważnie idą po prostu w parze. Więcej na ten temat można poczytać w książce Kenta Becka "Wzorce implementacyjne". Podsumowując, nie patrzymy na długość kodu czy szybkość jego działania a na to jak szybko inni programiści (lub my sami) jesteśmy w stanie kod zrozumieć i go zmodyfikować w razie potrzeby.

Generalnie kod powinien mieć 3 wartości: komunikatywność, prostota, elastyczność.

Dlaczego akurat tylko te trzy? Lista cech, którymi powinien charakteryzować się dobry kod, jest dużo dłuższa.

Podsumowując, nie patrzymy na długość kodu czy szybkość jego działania a na to jak szybko inni programiści (lub my sami) jesteśmy w stanie kod zrozumieć i go zmodyfikować w razie potrzeby.

Szybkość działania kodu jest bardzo istotna w pewnych zastosowaniach. Czas pracy programisty może wydawać się drogi, ale czas pracy użytkownika pomnożony przez milion użytkowników lub koszt pracy serwera pomnożony przez 10 tysięcy serwerów może dawać jeszcze większą kwotę.

"premature optimization is the root of all evil"

To jest niestety bardzo często używane jako wymówka słabych programistów nie potrafiących pisać wydajnego kodu za pierwszym podejściem (niekoniecznie optymalnego, ale przynajmniej łatwo-optymalizowalnego i niezawierającego WTFów wydajnościowych). Bardzo często dobre przemyślenie architektury, struktur danych i algorytmów na samym początku pod kątem wydajności powoduje, że otrzymujemy znacznie wydajniejszy program prawie bez wydłużania czasu trwania projektu. Nikomu nie życzę optymalizowania źle zaprojektowanego i naklepanego byle jak programu, który na 2 dni przed terminem okazał się 10x zbyt powolny.

Dlaczego akurat tylko te trzy? Lista cech, którymi powinien charakteryzować się dobry kod, jest dużo dłuższa.

Można tę listę uszczegółowić, ale w efekcie będzie się ona składać na te trzy wartości. Zresztą nie ja to wymyśliłem, tylko Kent Beck, ale w pełni się z nim zgadzam.

Szybkość działania kodu jest bardzo istotna w pewnych zastosowaniach. Czas pracy programisty może wydawać się drogi, ale czas pracy użytkownika pomnożony przez milion użytkowników lub koszt pracy serwera pomnożony przez 10 tysięcy serwerów może dawać jeszcze większą kwotę.
Czas pracy programisty nie wydaje się drogi - on jest drogi. Bottlenecki można znaleźć relatywnie szybko i sprawnie je zoptymalizować gdy kod jest łatwy w utrzymaniu i modyfikacji.
A optymalizowanie czegoś co póki co nie wykazuje żadnych złych objawów jest stratą czasu.

To jest niestety bardzo często używane jako wymówka słabych programistów nie potrafiących pisać wydajnego kodu za pierwszym podejściem (niekoniecznie optymalnego, ale przynajmniej łatwo-optymalizowalnego i niezawierającego WTFów wydajnościowych). Bardzo często dobre przemyślenie architektury, struktur danych i algorytmów na samym początku pod kątem wydajności powoduje, że otrzymujemy znacznie wydajniejszy program prawie bez wydłużania czasu trwania projektu. Nikomu nie życzę optymalizowania źle zaprojektowanego i naklepanego byle jak programu, który na 2 dni przed terminem okazał się 10x zbyt powolny.

Można od razu pisać optymalny wydajnościowo kod, ale nie kosztem czytelnego kodu i tylko jeśli mamy dość doświadczenia, żeby stwierdzić "tak, w tym miejscu na pewno jest bottleneck".

I jeszcze to co kolega napisał wcześniej:

Zasady optymalizacji Jacksona:

1. Nie optymalizuj
2. (dla ekspertów) jeszcze nie optymalizuj.

@quetzakubica jasne że w większości zastosowań tak jest, ale są też takie dziedziny jak HFT gdzie różnicę robią nawet niewielkie optymalizacje.

Można od razu pisać optymalny wydajnościowo kod, ale nie kosztem czytelnego kodu i tylko jeśli mamy dość doświadczenia, żeby stwierdzić "tak, w tym miejscu na pewno jest bottleneck".

@Krolik nie mówi raczej o bottleneckach i ich szukaniu ze szklaną kulą, a o zwykłym zdrowym rozsądku przy pisaniu kodu. Żeby potem nie bylo tak:
http://en.wikipedia.org/wiki/Joel_Spolsky#Schlemiel_the_Painter.27s_algorithm
Chodzi nie o to żeby rozkminiać niepotrzebnie jak coś zoptymalizować, tylko żeby wiedzieć że jest różnica pomiędzy użyciem LinkedList i ArrayList. Że to nie jest obojętne czy masz HashMap czy TreeMap.

Shalom napisał(a):

@quetzakubica jasne że w większości zastosowań tak jest, ale są też takie dziedziny jak HFT gdzie różnicę robią nawet niewielkie optymalizacje.

Można od razu pisać optymalny wydajnościowo kod, ale nie kosztem czytelnego kodu i tylko jeśli mamy dość doświadczenia, żeby stwierdzić "tak, w tym miejscu na pewno jest bottleneck".

@Krolik nie mówi raczej o bottleneckach i ich szukaniu ze szklaną kulą, a o zwykłym zdrowym rozsądku przy pisaniu kodu. Żeby potem nie bylo tak:
http://en.wikipedia.org/wiki/Joel_Spolsky#Schlemiel_the_Painter.27s_algorithm

Oczywiście, tu masz racje. W żadnym wypadku nie mówię o skrajnych przypadkach i ślepym stosowaniu wszystkiego co się przeczyta.

Bottlenecki można znaleźć relatywnie szybko i sprawnie je zoptymalizować gdy kod jest łatwy w utrzymaniu i modyfikacji.

Jak masz architekturę z d***, to żadna ilość "clean code", TDD i wzorców projektowych Ci nie pomoże i będziesz bottlenecki usuwał latami. Wystarczy, że ktoś wpadnie na genialny pomysł, aby np. wszelkie modyfikacje stanu systemu wymagały globalnej blokady, a później reszta systemu bazowała na tym założeniu. Zmiana takiego fundamentalnego założenia może być bardzo bolesna i równoważna niemal z przepisaniem systemu na nowo.

@Krolik, no trochę nie tak. Jeżeli taki globalny lock jest dobrze zaimplementowany m.in. spełnia te wszystkie złe Clean Cody, TDD i insze wzorce projektowe to jego usunięcie będzie stosunkowo proste. Może być długotrwałe, ale w dobrze zaprojektowanym systemie może oznaczać tyle co modyfikację kilku linijek w jednej klasie (która zarządza lockiem i jest wspólnym punktem dostępu dla wszystkich innych klas).

Zresztą, kod w pierwszej kolejności musi być czytelny. Nie dlatego, że będzie to fajne, ale dlatego, że późniejsze zmiany będą łatwiejsze do wprowadzenia. Dodatkowo jeżeli kod będzie wymagał tuningu, który spowoduje zmniejszenie czytelności to będzie można taką operację przeprowadzić w taki sposób by utrata czytelności była jak najmniejsza. Po prostu będzie wiadomo gdzie i co dokładnie stanowi problem i jego usunięcie nie będzie już trudne.

Krolik napisał(a):

Bottlenecki można znaleźć relatywnie szybko i sprawnie je zoptymalizować gdy kod jest łatwy w utrzymaniu i modyfikacji.

Jak masz architekturę z d***, to żadna ilość "clean code", TDD i wzorców projektowych Ci nie pomoże i będziesz bottlenecki usuwał latami. Wystarczy, że ktoś wpadnie na genialny pomysł, aby np. wszelkie modyfikacje stanu systemu wymagały globalnej blokady, a później reszta systemu bazowała na tym założeniu. Zmiana takiego fundamentalnego założenia może być bardzo bolesna i równoważna niemal z przepisaniem systemu na nowo.

Ale przecież mowa jest o czytelnym kodzie a nie o architekturze. Nikt nie powiedział, że zostawianie optymalizacji na później równa się doborowi złej architektury, to są dwie odrębne rzeczy. Trzeba zarówno pisać kod łatwy do zrozumienia jak i w dobrej architekturze. Zresztą te dwie rzeczy idą w parze (DDD). Jak wybierzemy złą architekturę (czyli najczęściej po prostu, nie zastosujemy żadnej), to kod będzie skomplikowany tak czy siak.

W haśle "optymalizacja" zawiera się optymalna architektura, optymalne struktury danych i optymalne algorytmy. Nie da się zostawić optymalizacji na później. Jeżeli projekt wymaga wydajności, to o optymalizacji trzeba myśleć od początku. A wydajny kod bardzo często daje się pisać w sposób bardzo czytelny - trochę nie rozumiem czemu uważacie, że to są przeciwstawne pojęcia. To nie jest albo-albo, bo można mieć jedno i drugie.

Trzeba zarówno pisać kod łatwy do zrozumienia jak i w dobrej architekturze. Zresztą te dwie rzeczy idą w parze (DDD). Jak wybierzemy złą architekturę (czyli najczęściej po prostu, nie zastosujemy żadnej), to kod będzie skomplikowany tak czy siak.

Z tym się zgadzam. Ja pisałem jednak o implikacji w drugą stronę. Że z czytelnego kodu wcale nie musi wynikać architektura dająca się łatwo optymalizowac pod kątem wydajności. Tak jak mówiłem, walnięcie globalnego locka na bazie danych bardzo mocno upraszcza kod i architekturę, ale wycofanie się z takiej decyzji może być bardzo bolesne, dlatego o takich rzeczach trzeba myśleć na początku, a nie zostawiać na koniec.

Dla mnie są takie hasła z tym związane:

1. czytelny kod
2. wydajny kod
3. optymalny kod
4. optymalizacja
5. optymalne struktury i algorytmy

Prosty i czytelny kod łatwo daje się zoptymalizować, ponieważ jest zrozumiały. Zawsze powinno się zaczynać od tej postaci. Być może nigdy nie trzeba będzie go optymalizować, a jeśli zajdzie taka potrzeba za pół roku to łatwo będzie to zrobić.

Wydajny (najwydajniejszy) kod to kod w którym wszystkie ścieżki już są zoptymalizowane i jest to tzw. "dead end" - nie da dalej się go rozwijać. W przypadku takiego kodu warto jest mieć kod referencyjny (czytelny). Tak robiło się AFAIR dla wstawek ASM w Delphi - jako dokumentację pisano odpowiednik w Delphi.

"Optymalny kod" (w sensie inżynieryjnym a nie wydajnościowym) to taki który w którym wydajność jest "wystarczająca" ale nadal jest czytelny / dobrze udokumentowany. Jest gdzieś pośrodku pomiędzy 1 i 2.

Optymalizacja to proces modyfikacji kodu z postaci 1 na postać 2. Wykonuje ją się gdy testy wydajnościowe pokazują że oprogramowanie nie spełnia wymaganych założeń.

O 5 należy pamiętać zawsze, ale najlepiej jest najpierw stosować rozwiązania popularne (w C++ to będzie STL) a w drugiej kolejności (po optymalizacji) rozwiązania super-wydajne z książek lub co gorsza własnoręcznie wymyślone.

W oryginalnym kodzie:

• argument "i" nic nie mówi i to na pewno jest do poprawy
• brakuje default w switch
• lepsze jest rozwiązanie "1", ponieważ w "2" jeśli jest tylko "else" to możliwe że ludzie zaczną stosować różne wartości != 1, np. 0,2,width+1,user_id itd. w związku z czym przy próbie dodania nowego kodu wartości "i" będzie to koszmar

Ciekawa dyskusja.
Ja w swoim projekcie (Delphi) miałem kiedyś funkcję w pierwszej wersji jak poniżej. Na fali optymalizacji i skracania kodu, popełniłem o wiele krótszą wersję drugą i tak zostało. Ale w związku z tym mam swego rodzaju kaca. Sam się tearz zastanawiam jak to działa.

function TInfra.GetMainBlockTopLeft(const AForm: TForm): TPoint;
var
a: integer;
begin
a := (AForm.Width - MAIN_BLOCK_DEF_WIDTH) div 2;
case FClockPos of
cp12:
begin
result.X := a - 50;
result.Y := 20;
FClockPos := cp3;
end;
cp3:
begin
result.X := a - 30;
result.Y := 40;
FClockPos := cp6;
end;
cp6:
begin
result.X := a - 50;
result.Y := 60;
FClockPos := cp9;
end;
cp9:
begin
result.X := a - 70;
result.Y := 40;
FClockPos := cp12;
end;
end;
end;

function TInfra.GetMainBlockTopLeft(const AForm: TForm): TPoint;
const
lShift: array[TClockPos] of integer = (-20, 0, 20, 0);
begin
result.Y := 40 + lShift[FClockPos];
FClockPos := TClockPos((1 + Ord(FClockPos)) mod (1 + Ord(High(TClockPos)))); // cyclic pass in enumeration
result.X := ((AForm.Width - MAIN_BLOCK_DEF_WIDTH) div 2) + lShift[FClockPos];
end;

@albi77:
W Twojej pierwszej wersji nie obsługujesz nieznanych (nowych) wartości TClockPos. Po dodaniu nowej wartości funkcja będzie zwracała nie wiadomo co.
W drugiej wersji powtarzasz fragment obliczeń.

Moja wersja - kombinacja tych dwóch wyżej.

function TInfra.GetMainBlockTopLeft(const AForm: TForm): TPoint;
const
    // kolejność do weryfikacji - zależy od definicji TClockPos
    xShift: array[TClockPos] of integer = (-50, -30, -50, -70);
    yPos: array[TClockPos] of integer = (20, 40, 60, 40);
    nextPos: array[TClockPos] of TClockPos = (cp3, cp6, cp9, cp12); 
var
    a: integer;
begin
    a := (AForm.Width - MAIN_BLOCK_DEF_WIDTH) div 2;
    result.X := a + xShift[FClockPos];
    result.Y := yPos[FClockPos];
    FClockPos := nextPos[FClockPos];
end;

W razie dodania nowej wartości program się nie skompiluje - błąd zostanie od razu wykryty.
Mogą być jakieś błędy składniowe, nie testowałem.

BTW. do poprawy jest nazwa funkcji - nie wskazuje na to że kolejne wywołania będą zwracały różne wartości.