Dzisiaj zamieszczam drugi post z serii Top 11. Tym razem – krótkie miniporady zwiększające czytelność i polepszające organizację kodu. Wszystkie bezdyskusyjnie stosowane przeze mnie i zdecydowanie sprawdzone. Zdaję sobie sprawę, że nie napiszę nic nadzwyczajnie odkrywczego, jednak jeśli chociaż kilku osobom pojawi się w głowie chmurka z napisem “faktycznie, można to tak zrobić” – to już będzie mój wielki sukces:).
Oczywiście to co zaprezentuję to najniższy poziom “usprawnień” – na tymże poziomie przykładowych pojęć “wzorce projektowe” czy “polimorfizm” nawet się nie bierze pod uwagę. I o to chodzi, bardziej skomplikowane rzeczy zasługują na osobne, tylko im poświęcone posty.
Zatem po kolei:
1) Regiony
Gdy jakiś czas temu zostałem brutalnie wrzucony w świat Javy – brak regionów uderzył we mnie z całą mocą. WTF? Jak się bez tego połapać w kodzie? Chwała pomysłodawcy regionów. A tym, którzy mogą ich używać, a nie używają – na pohybel! Dla zupełnie zielonych: gdy jakąś część kodu otoczymy dyrektywami
1: #region [NazwaRegionu]
2: #endregion
2) Usings
Piszmy DateTime zamiast System.DateTime, piszmy StringBuilder zamiast System.Text.StringBuilder, i tak dalej… A konieczne do zaimportowania przestrzenie nazw umieszczajmy na samej górze pliku, najlepiej w zwiniętej sekcji “usings”.
Kompilatorowi nie robi to różnicy, a czytającemu kod – jak najbardziej.
Oczywiście czasami są wyjątki od tej reguły, ale tak to już jest z regułami…
3) Zakomentowany kod jest be!
Nie komentujmy kodu – usuwajmy go! Zakomentowany kod może mieć sens przez kwadrans, może dzień, może dwa. Ale po dłużej chwili i tak nie będzie nadawał się do użytku. Nawet jeśli zawarta w nim funkcjonalność za X czasu okaże się jednak potrzebna, zdecydowanie lepiej jest napisać ją od nowa niż odkomentować stary kod, zagłębiać się w jego sens, bezmyślnie poprawiać metodą “kompiluje się, znaczy działa” a potem spędzać długie godziny nad szukaniem błędów, których w takich sytuacjach ciężko uniknąć.
Poza tym – mamy przecież repozytorium kodu, za pomocą którego bez problemu dostaniemy się do każdej napisanej, zedytowanej i usuniętej linijki.
4) Nazewnictwo
Porada bardzo krótka, jednak bardzo ważna: stosujmy nazwy tak, jak zalecane jest w dokumencie “.NET Framework General Reference – Naming Guidelines”. Bez zbędnych wynalazków, bez super-wymyślnych i mega-zrozumiałych własnych konwencji… Ten dokument to po prostu przyjęty i zaakceptowany standard (oczywiście w świecie .NET – smutek wynikły z nazywania metod z małej litery czy zostawiania nawiasów klamrowych w tej samej linii zostawmy Javowcom:)).
5) Operatory ?? i ?:
Pierwszy operator pozwala wyeliminować zbędne porównania do null:
1: // jest:
2: if (p == null)
3: return new object();
4: else
5: return p;
6:
7: // niechaj się przeistoczy w:
8: return p ?? new object();
Drugi operator z kolei skraca proste ify tak, by logicznie można je było zapisać w postaci jednej operacji:
1: // jest:
2: if (warunek)
3: return 1;
4: else
5: return 0;
6:
7: // niechaj się przeistoczy w:
8: return (warunek) ? 1 : 0;
6) Globals.cs, Common.cs, Shared.cs i inne SodomaIGomora.cs
Nie twórzmy plików, których przeznaczeniem jest przechowywanie wszystkiego, co nie pasuje nigdzie indziej! Ileż razy widzieliśmy klasę Globals operującą na datach, wykonującą konwersje między typami, pokazującą wiadomości w MessageBoxach…? Tylko yerba-mate nie parzy.
Oczywiście nie można uniknąć w systemie różnego rodzaju metod pomocniczych bez wielokrotnego ich implementowania (co jest jeszcze gorsze niż Śmietnik.cs). W takiej sytuacji starajmy się chociaż grupować podobne w operacje w specjalne klasy, na przykład w katalogu Utils niech znajdą się pliki DateTimeUtils.cs, MessageUtils.cs i tak dalej… Nadal nie wygląda to ślicznie jak Doda “The Bright Silicon Doll”, ale nie potrafię wymyślić lepszego sposobu. Może jakieś sugestie od czytelników?
7) Enum (typy wyliczeniowe – nie mylić z enumeratorem)
Tworząc enum mamy na myśli coś konkretnego – chcemy nadać jakiejś klasie pewną cechę, której wartości mają zawierać się w ściśle określonym zakresie. Niejednokrotnie można spotkać się z “enumami” porozrzucanymi po całym projekcie lub zgrupowanymi w pliku Enums.cs. A dlaczego? Przecież zwykle enum jest ściśle powiązany z jedną klasą lub jedną hierarchią klas. Dlatego definiujmy je wewnątrz owej klasy z modyfikatorem public, na przykład tak:
1: class Person
2: {
3: public enum PersonGender
4: {
5: Male,
6: Female
7: }
8:
9: public PersonGender Gender { get; set; }
10: }
8) GenerateMember = false
Kontrolki w Windows Forms mają właściwość GenerateMember. Domyślnie jej wartość to true, jednak zachęcam do zmiany na false wszędzie, gdzie to tylko możliwe.
Co to zmieni? Deklaracja kontrolki zostanie przeniesiona z ciała klasy do wnętrza metody InitializeComponent, dzięki czemu bezpośredni dostęp do niej z kodu stanie się niemożliwy.
Moim zdaniem taka powinna być domyślna wartość tej właściwości (albo powinna być chociaż możliwość zmiany domyślnej wartości z poziomu VS, czego nie udało mi się znaleźć). Dlaczego? Wówczas bardziej popularne byłoby intensywne wykorzystanie DataBinding (klasy BindingSource, BindingContext, CurrencyManager…) zamiast ręcznej synchronizacji danych <-> UI czy odwoływanie się do parametru sender z obsługi zdarzeń zamiast bezpośrednio do konkretnej kontrolki.
Oczywiście pewnych problemów nie przeskoczymy, i wtedy nie obejdzie się bez posiadania danego elementu UI w postaci pola klasy, ale takie sytuacje to wyjątki.
9) Aliasy C#
Język C# posiada aliasy dla większości (wszystkich?) typów prymitywnych oraz kilku nieprymitywnych. Korzystajmy z nich: używajmy int zamiast System.Int32, float zamiast System.Single, string zamiast System.String czy object zamiast System.Object – to dla naszej własnej wygody.
10) Warunki kończące metodę
Jeżeli wykonanie czynności zawartych metodzie zależy od pewnych warunków – sprawdźmy ich negację i wyjdźmy z metody, jeśli zachodzi taka potrzeba. Unikniemy pociętego, “potabowanego”, nieczytelnego kodu. Dla zobrazowania mały przykład:
1: // jest:
2: metoda()
3: {
4: if (warunek)
5: {
6: instrukcja1();
7: instrukcja2();
8: }
9: }
10:
11: // niechaj się przeistoczy w:
12: metoda()
13: {
14: // bonus: po konferencji C2C pomyslałem chwilę nad tą dokładnie linijką i zgadzam się z Pawłem Leśnikowskim prowadzącym sesję o TDD – piszemy warunek == false zamiast !warunek, bo tak jest lepiej i koniec
15: if (warunek == false)
16: return;
17:
18: instrukcja1();
19: instrukcja2();
20: }
11) var…
To słowo kluczowe dodane w C# 3.0 jest dla programisty kuszące niczym dla Adama jabłko nadgryzione przez nagą Ewę. Ba, Resharper 4 domyślnie bardzo usilnie namawia nas nawet do stosowania go przy deklaracji KAŻDEJ zmiennej lokalnej! Ja bym był jednak ostrożny – i zalecam jego użycie tylko w scenariuszach związanych z LINQ i typami anonimowymi oraz w definicji pętli foreach.
To 11 banalnych porad, które moim zdaniem mogą dość znacząco wpłynąć na przejrzystość pisanego kodu. Bardzo chętnie poznam Wasze opinie o przedstawionych poglądach i poznam więcej podobnych zasad. Co robicie, aby Wasz kod był czytelny i ładny?
Z większością rzeczy się zgadzam, ale pozwól, że wtrące swoje małe "ale" :)
Ad 1) – jakiś czas temu zacząłem używać regionów na potęgę (regioneratethis rządzi!). Jednak warto imho tutaj być ostrożnym. Regiony pozwalają nadać kodzie blasku , który niekoniecznie semantycznie do końca może być tak efektowny. Chodzi mi o to, żeby nie wdać się w automatyczne "zamykanie" kodu, który uważamy za nie do końca dopracowany. "Zamknę go w region i bedzie cacy" – spowoduje poczucie, że klasa wygląda schludnie i zwarto. Osobiście zaprzestałem używania regioneratethis z tego własnie powodu a preferuje widok classview (lub FileStructure jeśli ktoś używa R#). Natomiast jeśli obsługujemy np. wszystkie eventy wywoływane przez np. GridView to …bez regionów ciężko:) Krótko: regionów uzywać, ale z głową i nie dać im się omamić:)
Ad 6) – Dodam tylko, że w takich przypadkach teraz też mamy Extension Methods – warto używać.
Reszta, a w szczególności (11) (- mam identyczne spostrzeżenie) warta zapamietania.
Aha, drobna sugestia, mógłbyś zmienić kolor komentarzy na ciut jaśniejszy? Światło mi z rana intensywnie pada na monitor i dopiero pod koniec zauważyłem w ogóle, że są:)
– 1) jasne że trzeba ich używac z głową i nie traktować jako narzędzia do chowania syfu przed wrażliwymi oczami:)
– 6) tak, Extension Methods są git – ale je też trzeba gdzieś trzymać:) i tu istnieje ryzyko wrzucenia ich do SyfKiłaIMogiła.cs
Dzięki za uwagę co do koloru komentarzy, dziś wieczorem postaram się to zmienić.
Keep it coming!
Mam kilka komentarzy:
Ad 1) Osobiście staram się umieszczać opis regionu również po dyrektywie #endregion, w ten sposób:
#region <Opis>
#endregion <Opis>
Przy rozwiniętych regionach minimalnie polepsza to czytelność kodu.
Ad 4) Podlinkowana wersja dokumentu ".NET Framework General Reference – Naming Guidelines" dotyczy .NET Framework 1.1 (hint: VS.[i]NN[/i] w adresie oznacza wersję dokumentu. VS.71 oznacza .NET 1.1, VS.80 – .NET 2.0, VS.85 – .NET 3.0, VS.90 – .NET 3.5. Można usunąć nawias i wtedy dostaniemy najnowszą wersję. Akurat ten konkretny dokument dotyczy wyłącznie .NET 1.1). Aktualna wersja jest dostępna pod adresem http://msdn2.microsoft.com/en-us/library/ms229002.aspx.
Ad 5) Wszelkiego rodzaju skrócone notacje to najczęściej broń obosieczna. Szczególnie skrócona notacja warunkowa, osobiście nie polacałbym jej nikomu w ramach best practices. Wygląda to świetnie na papierze, ale z moich obserwacji wynika, że stosowanie jej, choć zwiększa zwięzłość, najczęściej wpływa negatywnie na czytelność kodu. Próbujemy w jednej linii zmieścić praktycznie trzy wyrażenia, więc często zdarza się, że całe wyrażenie wraz z odstępami jest szersze, niż szerokość ekranu, nawet, jeśli jest bardzo proste (ot np. gdy wywołujemy w nim metodę, która przez przypadek ma bardzo wylewną nazwę).
Spotkałem się z takim zapisem:
if (w != null) {
return w.Object;
}
return GetDefaultObject();
i to jest według mnie w miarę zwięzłe (średnio 2, 3 linie mniej, niż gdy stosujemy else), a już na pewno bardzo czytelne.
Ad 7)
[b]DO NOT[/b] use public nested types as logical grouping construct; use namespaces for this.
[b]AVOID[/b] publicly exposed nested types. The only exception to this is if variables of the nested type need to be declared in rare scenarios such as subclassing or other advanced customization scenarios.
[b]DO NOT[/b] use nested types if the type is likely to be referenced outside of the containing type.
Źródło: Krzysztof Cwalina, Brad Adams, [i]Framework Design Guidelines: Conventions, Idioms and Patterns for Reusable .NET Libraries[/i]
Ja się zgadzam z autorami w tej kwestii – definiowanie zagnieżdżonych typów wyliczeniowych w żadnym wypadku nie jest najlepszą praktyką, typy nie służą do grupowania. Jeśli spróbujemy skonsumować taki typ wyliczeniowy gdzieś w kodzie, dostaniemy:
Person.PersonGender gender = Person.PersonGender.Female;
a z czymś takim zdaje się próbowaliści walczyć w punkcie 2). Tu sytuacja jest trochę gorsza, bo nie pomoże nam prosty prosta deklaracja użycia przestrzeni nazw (w końcu Person nią nie jest). Możemy skorzystać jedynie z aliasów:
using Gender = Person.PersonGender;
…
Gender gender = Gender.Female;
ale:
a) Visual Studio w żaden sposób nie zautomatyzuje nam tworzenia takiego aliasu (w przypadku przestrzeni nazw wystarczyło wpisać nazwę typu, a resztę załatwiał smart tag).
b) Rozpętamy piekło. Jeden programista utworzy alias Gender, drugi Sex, a trzeci, który będzie to potem czytać i który przypadkiem jest autorem oryginalnego kodu, będzie się zastanawiać: [i]Gdzie ja u licha zadeklarowałem typ "Sex"?[/i].
Osobiście w przypadku, gdy typ wyliczeniowy jest powiązany z jedną klasą, definiuję go po prostu w tym samym pliku, co klasę, najlepiej jeszcze przed jej definicją. I to jest rozwiązanie podobne do Twojego, ale pozbawione jego wad.
Ad 11) Nie widzę przeciwwskazań również w przypadku, gdy inijcjalizujemy zmienną nowo utworzonym obiektem, np.:
StringBuilder sb = new StringBuilder();
— vs —
var sb = new StringBuilder();
(albo coś bardziej życiowego)
ConnectionEndPointSecurityInformationProvider provider = new ConnectionEndPointSecurityInformationProvider()
— vs —
var provider = new ConnectionEndPointSecurityInformationProvider();
Dzieki Olek za spostrzeżenia – poprawiłem link w poście, wcześniej nie zwróciłem na to uwagi.
Może dorzucisz jeszcze jakieś wskazówki? Miło poczytać jakie przyzwyczajenia ma "człowiek, który wie wszystko" :).
Znam dwie osoby, które wiedzą wszystko, jedną z nich jest Tomek Kopacz, a żadną z nich nie jestem ja. I wiem, że naprawdę sporo mi do nich brakuje. Zresztą spytaj Kuby J., zagiął mnie raz pytaniem :). Co do moich programistycznych przyzwyczajeń, to pewnie jest tego cała masa, ale tak na szybkiego nic mi nie przychodzi do głowy. O, wiem. Nigdy nie piszę "this.". Nigdy nie używam SCREAMING_CAPS w nazwach stałych. Stosuję nadenkapsulację, tj. enkapsuluję wszystko i wszędzie i staram się nigdy nie odwoływać do prywatnych pól spoza metod dostępowych lub konstruktora.
Mam kilka uwag technicznych odnośnie mechanizmu komentowania na blogu. Czy dałoby radę pozbyć się wcięcia na początku pierwszego akapitu? Jest trochę jak Hiszpańska Inkwizycja, nikt się go nie spodziewa. I czy dałoby radę wprowadzić dwa nowe znaczniki formatowania:
, zamieniany na <pre>[i]tekst[/i]</pre> oraz [url=[i]adres[/i]][i]tekst[/i][/url], zamieniany na <a href="[i]adres[/i]">[i]tekst[/i]</a>? Tych dwóch funkcji szczególnie mi brakuje.
Na koniec jeszcze jedna rzecz, którą chciałem skomentować, a jakoś mi umknęła wcześniej:
Ad 10) Zapisywanie warunków w ten sposób:
if (warunek == false) return;
może przynieść więcej szkody, niż pożytku. Wyrażenia logiczne, w których lewa strona porównania jest typu logicznego i jest l-wartością, są jako jedyne w całym języku C# podatne na coś, co można nazwać błędem nieumyślnego przypisania w warunku:
// Jakiś skomplikowany warunek.
bool canAdd = … ;
// Warunek będzie zawsze ewaluowany do false – na pewno tego chcieliśmy?
if (canAdd = false) throw new IvalidOperationException("Item cannot be added to the collection.");
Takie błędy są szalenie trudne do zauważenia, szczególnie w C#, gdzie praktycznie ciężko jest je popełnić.
Jeśli koniecznie chcemy mieć binarny operator i literał typu bool w warunku (choć ja nie widzę sensu), możemy zabezpieczyć się przed efaktami ubocznymi korzystając z klasycznego rozwiązania znanego z C++:
if (false == canAdd) throw new IvalidOperationException("Item cannot be added to the collection.");
Teraz wszystko jest niby OK, ale składnia ta jest na tyle niecodzienna w C#, że u nieprzyzwyczajonego czytelnika może powodować przestoje przy czytaniu kodu.
Ad 1)
Ja używam jeszcze innej konwencji z regionami:
#region <Opis>
#endregion //<Opis>
Dla mnie takie użycie daje najlepszą czytelność przy rozwiniętych regionach – bo mam nazwę regionu i od razu po kolorze rozróżniam czy to początek czy koniec.
Ad 10)
Ja mimo wszystko się nie zgodzę co do (warunek == false), z resztą zdziwiło mnie właśnie na C2C, że Paweł tak pisał, bo taki kod bardzo łatwo prowadzi do magicznych kwiatków w stylu
if (warunek == false)
return false;
else
return true;
Ad 5) Te akurat lubię bardzo, a chciałem tylko dodać do ?? że można to sobie rozwinąć jeszcze bardziej, np.:
int? x = null;
int? y = null;
int? z = null;
int? aa = 5;
int? cosik = x ?? y ?? aa ?? z; // co zwróci, a?
Ad 10)
if-a nie polecam, lepszy jest if (warunek), zbyt często robiło mi się if (warunek = true) ;)
Oprócz 7 punktu reszta jest codziennością. :) Co do 7 nie zagnieżdżam typów. :)
@apl: co do "człowieka który wie wszystko" – zobaczyłem to tutaj: http://www.codeguru.pl/forum-posts-12379-0.aspx#40595 i strasznie mi się spodobało:). A wcięcie przy komentarzu czy znaczniki formatowania… wziąłem co daje BlogEngine.NET i się nie zagłębiałem w szczegóły, ale przyznaję że rozjechany komentarz może lekko irytować. Brak [url] też, o czym przekonałem się dopiero teraz wklejając po raz pierwszy linka.
@leszcz: faktycznie fajny motyw z komentarzem po #endregion, lepsze oznaczenia out-of-the-box
@jacek: z tym ?? to już przegiąłeś:) wygląda jak na egzaminie u podirytowanego 50-letniego kawalera prowadzącego laborki z programowania któremu zdechła rybka… Pisząc o operatorach w 5) miałem na myśli naprawdę proste konstrukcje, to co przedstawiłeś może przyprawić o niezły zawrót głowy osobę czytającą kod, a nie o to chodzi. Błąd wynikający z zapisu: "warunek = true" można z kolei wyeliminować zamieniając kolejność, jak napisał apl, co faktycznie jest dobrym pomysłem.
10)
a ja zawsze pisze !warunek bo tak jest jednak lepiej.
latwiej sie czyta i nie ma problemow z = zamiast ==.
koniec.
Do if (warunek = false) dodam jeszcze, że w takiej sytuacji otrzymamy ostrzeżenie kompilatora (CS 0665) http://msdn2.microsoft.com/en-us/library/c1sde1ax.aspx .
@lpodolak, apl:
Zmieniłem kolor komentarzy, pól do ich dodawania oraz dodałem znacznik code zamieniany na pre.
Wcięcia w komentarzach (widoczne nie we wszystkich przegladarkach…) i znacznik url to nie kwestia minuty i na razie jest jak jest.
Coś jest nie tak, jak patrzę na mój komentarz z 10 kwietnia 2008 15:37, to on się już podporządkował nowemu formatowaniu, ale z niewiadomych przyczyn tekst za blokiem pre jest pomniejszony.
Spróbujmy zreprodukować:
Tekst przed blokiem pre
[pre]Tekst wewnątrz bloku pre[/pre]
Tekst za blokiem pre
Tfu, blokiem code :)
Tekst przed blokiem code
Tekst za blokiem code
Pobawiłem się Firebugiem i wiem, z czego wynika wcięcie w pierwszym akapicie. W każdym komentarzu znajduje się coś takiego:
Klasa gravatar ma następującą definicję CSS:
.comment .gravatar { float:left; margin:5px 10px 5px 0pt; }Wszystko przez float:left, powoduje on powstanie kwadracika 10x10px, który jest otaczany tekstem. Wydaje mi się, że ten element nie jest w ogóle używany (to jest chyba miejsce na awatar), więc możesz rozwiązać problem dopisując do definicji stylu:
Oczywiście masz rację:). Zmieniłem, dzięki.
Jets sobie zmienna:
Jaki bedzie lepszy sposob na przekonwertowanie f na doubla, ktory przyjmnie wartosc 0 jesli f jest nullem?
czy
pomylka, w przypadku nulla miala byc wartosc 1, a wiec:
double d1 = System.Convert.ToDouble(f ?? 1);
czy
double d2 = Double.Parse(f == null ? "1" : f.ToString());
Wg mnie zdecydowanie Convert. Oczywiście trzeba zabezpieczyć się przed wyjątkami. I dodałbym jeszcze drugi parametr – IFormatProvider (NumberFormatInfo.InvariantInfo, NumberFormatInfo.CurrentInfo lub własną konstrukcję).