Cena / wydajność

Dzisiaj będzie krótko, bo zajęty jestem i więcej czytam niż piszę. Także taka krótka refleksja:
Każdy wie, że koszta trzeba minimalizować. Uważam jednak, że istnieje pewna granica tych ograniczeń.
Zastanówmy się pracujecie z jakimś narzędziem i jest spoko, robi to czego się od niego oczekuje. Ale moglibyście pracować z innym, za które trzeba zapłacić i nie są to jakieś groszowe sprawy. Kwota którą pracodawca płaci nam to też nie są grosze, a kwota którą otrzymuje za naszą pracę od docelowego klienta to na pewno sensowne pieniądze. Dobrze mówię? Do czego zmierzam? Załóżmy że dobre narzędzie potrafi z każdej godziny naszej pracy wycisnąć jeszcze pięć minut więcej. Zakładam dla równych obliczeń, że pracownik za swoją prace (168 godzin) dostanie pięć tysięcy polskich złotych wynagrodzenia. Jasne – wiem, że z tym różnie bywa. Ale tu chodzi o okrągłe liczby. Co nam to daje?
60 minut pracy plus lepsze narzędzie to dodatkowe 5 minut.
60 minut to godzina, dzień pracy to 8 godzin, czyli dodatkowe 40 minut dla pracodawcy.
Tydzień ma 5 dni roboczych czyli 200 darmowych minut dla pracodawcy.
Pozwolę sobie dodać 10 minut, żeby wyszło 3.5h w tygodniu.
Pracodawca płaci około 30 złoty (5000/168) za godzinę pracy. Tak naprawdę to pewnie więcej, bo patrzę na to z perspektywy pracownika, który taką kwotę otrzyma przelewem na konto.
Po ponownym zaokrągleniu i pomnożeniu daje 100 złoty zaoszczędzone w tydzień, jeśli pracujemy z lepszym narzędziem.
Miesiąc to średnio cztery tygodnie czyli 400 złoty.
Znowu dla prostoty rachunków proponuje przejście na okrągłe kwoty i przyjmę kwotę 100 dolarów.

Krótkie zebranie myśli: zakładam że dobre narzędzie potrafi poprawić moją (twoją) pracę w taki sposób, że w trakcie każdej godziny mojej pracy, potrafię być o pięć minut wydajniejszy, co powoduje, dodatkowy zysk dla pracodawcy 100 dolarów w skali miesiąca. Przestańcie czytać teraz jeśli się nie zgadzacie.

Co to znaczy? O ile wszystko dobrze rozumiem i dobrze policzyłem, kupienie normalnej wersji Visual Studio zacznie zwracać się po pięciu miesiącach. VS2010 wersja professional kosztuje 499$. Dodatkowo kupienie R#, Telerika, czy Tomato to kwota zaczynająca się od 99$ za licencję. Załóżmy w takim razie, że po pół roku cała inwestycja zaczyna się zwracać.
Pięć minut do których ciągle wracam, to tylko czas poświęcony na napisanie kodu, nie liczę już poprawienia czytelności kodu, czy późniejszego utrzymywania kodu, pisania testów, etc. podczas których te narzędzia, także w dużej mierze wspomagają programistów.
Dodatkowo im ważniejszy pracownik, wyższa pensja tym droższy jest jego czas i należy go lepiej wykorzystywać.

To co mnie najbardziej zastanawia, to dlaczego większość firm (chyba że to tylko moje doświadczenie) nie inwestuje w dobre narzędzia, które czynią ich pracowników lepszymi programistami? Oczywiście że można korzystać z Eclipsa (nie przepadamy za sobą), SharpDevelop, czy NetBeans, czy wreszcie z VS w wersji Express, są dostępne za darmo i zrobią to czego się od nich oczekuje: edycja + kompilacja. Tylko czy warto? Czy naprawdę będzie to oszczędność?

Założenia dotyczące czasu czy wynagrodzenia są wzięte z głowy i oczywiście mocno zależą od tego co się robi, gdzie, w jakiej technologii, etc. Mają na celu tylko zobrazowanie jak tanie są narzędzia w porównaniu do czasu spędzonego nad projektem przez jednego człowieka.

SOLIDnie po łebkach

Ilu z nas wracając do starego kodu (napisanego wczoraj/ tydzień temu/ miesiąc temu) krzywi się patrząc na bałagan, który po sobie pozostawił? W zasadzie prościej będzie chyba zapytać komu się to nie zdarza. Otóż amerykańscy naukowcy znaleźli na to sposób. No dobra może nie amerykańscy, ale skrót jest z angielskiego – SOLID, rozkłada się on na pięć czynników, a każdy z nich jest znowu jakimś skrótem.

S – (SRP) Single Responsibility Principle
O – (OCP) Open / Closed Principle
L – (LSP) Liskov Substitution Principle
I – (ISP) Interface Segregation Principle
D – (DIP) Dependency Inversion Principle
(podobno wszystko co napisane Helvetica jest mądrzejsze)

O co chodzi z tymi regułami wchodzącymi w skład SOLID? Jest to pięć zasad mówiących o tym jak tworzyć kod, który będzie łatwiej utrzymywać, rozszerzać, czytać, a problem głodu na świecie zniknie. Pitu pitu, już pokazuje przykłady, które będą proste, czasem wręcz naiwne, chodzi o ukazanie idei, nie rozwiązanie jakiegoś rzeczywistego problemu.

Single Resposibility Principle wszyscy piszą że “Nigdy nie powinno być więcej niż jednego powodu do modyfikacji”. Strasznie mnie irytuje taki zapis, bo można napisać “klasa jest odpowiedzialna tylko za jedną logiczną funkcjonalność”. Skoro służy do mierzenia, to nie służy do zapisywania. Jeśli ma konwertować wartości z jednego formatu na inny, to nie powinna ich wysłać do innych części aplikacji. etc., itd., itp.

.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: Consolas, “Courier New”, Courier, Monospace;
background-color: #ffffff;
max-height: 300px;
overflow: auto;
/*white-space: pre;*/
}

.csharpcode pre { margin: 0em; }

.csharpcode .rem { color: #008000; }

.csharpcode .kwrd { color: #0000ff; }

.csharpcode .str { color: #a31515; }

.csharpcode .op { color: #0000c0; }

.csharpcode .preproc { color: #cc6633; }

.csharpcode .asp { background-color: #ffff00; }

.csharpcode .html { color: #800000; }

.csharpcode .attr { color: #ff0000; }

.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}

.csharpcode .lnum { color: #606060; }

   1:   public class BadHardwareDevice
   2:      {
   3:          int hardwareReadingLevel;
   4:          void DisplayHardwareLevelAlarm()
   5:          {
   6:              Console.WriteLine("Current value of {0} is dangerous!", hardwareReadingLevel);
   7:          }
   8:   
   9:          int GetHardwareReadings()
  10:          {
  11:              // reads data from device into hardwareReagingLevel
  12:              // return value from that reading
  13:              return hardwareReadingLevel;
  14:          }
  15:      }

Prosta klasa, ale zajmuje się ona dwiema rzeczami jednocześnie; odczytuje wartość z urządzenia (GetHardwareReadings) oraz wypisuje alarm na konsolę (DisplayHardwareLevelAlarm).
Kod można zmodyfikować na przykład w taki sposób:

   1:  public class BetterHardwareDeviceConsoleLogger
   2:      {
   3:          void DisplayHardwareLevelAlarm(int aLevel)
   4:          {
   5:              Console.WriteLine("Current value of {0} is dangerous!", aLevel);
   6:          }
   7:      }
   8:   
   9:      public class BetterHardwareDeviceReader
  10:      {
  11:          int hardwareReadingLevel;
  12:          int GetHardwareReadings()
  13:          {
  14:              // reads data from device into hardwareReagingLevel
  15:              // return value from that reading
  16:              return hardwareReadingLevel;
  17:          }
  18:      }

Teraz klasa odczytu zajmuje się tylko odczytem, natomiast klasa logująca alarm tylko wypisaniem na konsolę. Dodatkowo jeśli będzie potrzeba wysłania zapisu gdzieś indziej niż na konsolę nie będzie to miało wpływu na działanie innych komponentów. Bez modyfikacji już istniejących klas można stworzyć klasę BetterHardwareDeviceNetworkLogger i wysłać informację o poziomie wartości w świat.

Open / Close Principle wszyscy piszą, że klasa powinna być otwarta na rozszerzenia, a zamknięta na modyfikacje. I bądź mądry. Już nadciągam z chłopską odsieczą i rozumowaniem. Klasa powinna być tak napisana, aby nie trzeba było jej aktualizować gdy pojawi się nowa implementacja np. klasy logującej. Rozszerzenie funkcjonalne aplikacji powinno zostać zaimplementowane poza klasą, ona sama natomiast może z niej skorzystać. Jak zawsze bez kodu ciężko jest tłumaczyć:

   1:  public class BadLogger
   2:      {
   3:          public void LogMessage(string aMessage, BadLogTarger aTarget)
   4:          {
   5:              switch (aTarget)
   6:              {
   7:                  case BadLogTarger.ConLog:
   8:                      // write to console
   9:                      break;
  10:                  case BadLogTarger.NetLog:
  11:                      // write to network
  12:                      break;
  13:                  case BadLogTarger.DevNullLog:
  14:                      // ignore writing
  15:                      break;
  16:              }
  17:          }
  18:      }
  19:    
  20:      public enum BadLogTarger
  21:      {
  22:          ConLog,
  23:          NetLog,
  24:          DevNullLog,
  25:      }

Tutaj aby dodać logowanie np. do pliku należy dodać kolejnego enuma (przewińcie przykładowy kod na dół), a następnie rozszerzyć metodą LogMessage o odpowiedniego case….. dużo roboty. Może lepiej będzie zrobić coś takiego:

   1:  public class BetterLogger
   2:  {
   3:      ILogger logger;
   4:      public BetterLogger(ILogger aLogger)
   5:      {
   6:          this.logger = aLogger;
   7:      }
   8:      
   9:      public void LogMessage( string aMessage )
  10:      {
  11:          logger.LogMessage(aMessage);
  12:      }
  13:  }
  14:   
  15:  public interface ILogger
  16:  {
  17:      void LogMessage(string aMessage);
  18:  }
  19:   
  20:  public class ConLogger: ILogger
  21:  {
  22:      void ILogger.LogMessage(string aMessage)
  23:      {
  24:          // log to console
  25:   
  26:      }
  27:  }
  28:   
  29:  public class NetLogger : ILogger
  30:  {
  31:      public void LogMessage(string aMessage)
  32:      {
  33:          // log to network
  34:      }
  35:  }
  36:   
  37:  public class DevNullLogger : ILogger
  38:  {
  39:      public void LogMessage(string aMessage)
  40:      {
  41:          // log to network
  42:      }
  43:  }

Ojej więcej kodu. Tak tak, ale teraz obczaj to (miły czytelniku), po dodaniu nowego logera (np. FileLoger) implementuje się metodą LogMessage, a następnie przekazują taką klasę do BetterLogger, której nie trzeba już modyfikować. Wszystko działa tak jak poprzednio, a jest nowa funkcjonalność. Klasa jest otwarta na rozszerzenia, ale zamknięta na zmiany – voila.

Liskov Substitution Principle wszyscy strasznie komplikują opis. Mam proste wytłumaczenie tej zasady (proste bo po co komplikować, albo proste bo nie zrozumiałem zasady), jeśli dziedziczysz lub implementujesz pewien interfejs, wszystkie klasy pochodne powinny zachowywać się w podobny (logicznie podobny) sposób. Tak aby obiekt wykorzystujący referencję (wskaźnik) do klasy bazowej, mógł spokojnie wykorzystać funkcjonalność klas bardziej pochodnych i nie zostać zaskoczonym przez nietypowym zachowaniem (patrz kod poniżej). Dodatkowym założeniem jest, że parametry akceptowane przez klasy pochodne mogą być mniej restrykcyjne niż w klasie bazowej (może obsłużyć więcej przypadków), natomiast wartości zwracana mogą być takie same lub mniejsze (nie można zwrócić czegoś, co nie jest określone przez klasa bazowa). Dygresja: Przykład z prostokątem i kwadratem dla mnie ma sens i jest poprawny, nie wiem dlaczego jest uznawany za błędną implementacje, jeśli ktoś potrafi to wytłumaczyć proszę o kontakt.

   1:  public interface BadISettings
   2:  {
   3:      void Save();
   4:      void Load();
   5:  }
   6:   
   7:  public class BadUserSettings : BadISettings
   8:  {
   9:      public void Save()
  10:      {
  11:          // save user settings
  12:      }
  13:   
  14:      public void Load()
  15:      {
  16:          // load user settings
  17:      }
  18:  }
  19:   
  20:  public class BadApplicationSettings : BadISettings
  21:  {
  22:      public void Save()
  23:      {
  24:          // throw InvalidOperationException
  25:          // cannot overwrite application settings
  26:      }
  27:   
  28:      public void Load()
  29:      {
  30:          // load application settings
  31:      }
  32:  }

Co jest nie tak? Otóż ustawienia aplikacja nie chętnie się zapisują. Co wpływa na możliwość ich wykorzystania przez interfejs BadISettings. Interfejs nie definiuje nigdzie, że będzie rzucał wyjątkiem, poza tym nie definiuje on metody tylko po to żeby jej nie wspierać. Jednym z rozwiązań jest coś takiego:

   1:  public interface BetterIReadSettings
   2:  {
   3:      void Read();
   4:  }
   5:   
   6:  public interface BetterIWritableSettings
   7:  {
   8:      void Save();
   9:  }
  10:   
  11:  public class BetterUserSettings : BetterIReadSettings, BetterIWritableSettings
  12:  {
  13:      void Read()
  14:      {
  15:          // read settings
  16:      }
  17:   
  18:      void Save()
  19:      {
  20:          // save settings
  21:      }
  22:  }
  23:   
  24:  public class BetterApplicationSettings : BetterIReadSettings
  25:  {
  26:      public void Read()
  27:      {
  28:          // load application settings
  29:      }
  30:  }

//Pozdrawiam czułe oko Jacka

Teraz można spokojnie wykorzystać wspólny interfejs BetterIWritableSettings bez obaw że coś się wywali lub wyleci w powietrze. A BetterIReadableSettings z jedną metodą? A czy jest w tym coś złego? Zerknij na SRP.

Interface Segregation Principle – różnie o tym piszą, akurat nie ma informacji w rodzimym języku. Klienci naszej klasy nie powinni mieć dostępu do elementów z których nie korzystają. Będzie lepiej jeśli nawet nie będzie o nich wiedzieć. Klient powinien dostać tylko tyle ile potrzebuje, nic więcej. Jak zwykle abstrakcja i interfejsy są jor friend.

   1:  public class BadPerson
   2:  {
   3:      public string FirstName;
   4:      public string LastName;
   5:      public string EmailAddress;
   6:      public string PhoneNumber;
   7:      public string Address;
   8:  }
   9:   
  10:  public class BadEmailer
  11:  {
  12:      public void SendEmail( BadPerson aBadPerson )
  13:      {
  14:          // send email using just email address
  15:      }
  16:  }
  17:   
  18:  public class PhoneCaller
  19:  {
  20:      public void PhoneCall( BadPerson aBadPerson )
  21:      {
  22:          // make phone call using just phone number
  23:      }
  24:  }
  25:   
  26:  public class LetterSender
  27:  {
  28:      public void SendLetter (BadPerson aBadPerson )
  29:      {
  30:          // send letter using firstname, lastname and an address
  31:      }
  32:  }

Tutaj widać że klasa BadPerson agreguje wszystko co dotyczy jakiegoś człowieczka, co dał się wbić w internet za kubek kawy ;). Następnie wszystkie te informacje są przekazywane do innych obiektów, który wykorzystują tylko część z tych danych. Kto wie co one tak naprawdę robią z resztą z nich? Czy nie lepiej zamienić to na coś takiego:

   1:  public interface BetterIEmailable
   2:  {
   3:      string EmailAddress{get;set;}
   4:  }
   5:   
   6:  public interface BetterIPhoneable
   7:  {
   8:      string PhoneNumber{get;set;}
   9:  }
  10:   
  11:  public interface BetterILetterable
  12:  {
  13:      string FirstName{get;set;}
  14:      string LastName{get;set;}
  15:      string Address{get;set;}
  16:  }
  17:   
  18:  public class BetterPerson : BetterIEmailable, BetterILetterable, BetterIPhoneable
  19:  {
  20:      public string EmailAddress { get; set; }
  21:      public string PhoneNumber { get; set; }
  22:      public string FirstName { get; set; }
  23:      public string LastName { get; set; }
  24:      public string Address { get; set; }
  25:  }
  26:   
  27:  public class BetterEmailer
  28:  {
  29:      public void SendEmail(BetterIEmailable aBetterPerson)
  30:      {
  31:          // send email using just email address
  32:      }
  33:  }
  34:   
  35:  public class BetterSender
  36:  {
  37:      public void SendLetter(BetterILetterable aBetterPerson)
  38:      {
  39:          // send letter using firstname, lastname and an address
  40:      }
  41:  }
  42:   
  43:  public class BetterCaller
  44:  {
  45:      public void PhoneCall(BetterIPhoneable aBetterPerson)
  46:      {
  47:          // make phone call using just phone number and first name
  48:      }
  49:  }

Teraz każdy dostaje tylko to czego potrzebuje. Nie zaistnieje problem, w którym klasa wysyłająca pocztę elektroniczną zacznie zapisywać dane teleadresowe, podczas gdy miała wysłać tylko email. Wiecie rozumiecie.

Dependency Inversion Principle – mówi o tym, aby powiązania pomiędzy klasami były luźne. Klasy nie powinny zależeć od właściwej implementacji, a działać na wcześniej zdefiniowanym interfejsie. Dzięki temu prościej jest rozszerzać funkcjonalność lub całkowicie ją zmieniać, oczywiście w ramach interfejsu i zasady LSP.

   1:  public class BadButton
   2:  {
   3:      public BadButton( BadLamp aLamp)
   4:      {
   5:          Lamp = aLamp;
   6:      }
   7:   
   8:      public void Click()
   9:      {
  10:          this.Lamp.Switch();
  11:      }
  12:   
  13:      public BadLamp Lamp { get; private set; }
  14:  }
  15:   
  16:  public class BadLamp
  17:  {
  18:      readonly BadButton button;
  19:      public BadLamp()
  20:      {
  21:          this.button = new BadButton(this);
  22:      }
  23:   
  24:      public void Switch()
  25:      {
  26:          // switch the light on/off
  27:      }
  28:  }

Przykład powyżej to wyjątkowo mocne powiązanie klasy BadButton oraz BadLamp. Nie będą one współpracować z żadną inną klasą niż te zdefiniowane, chyba że dziedziczące po BadLamp, natomiast już na pewno nie zaistnieje możliwość zmiany guzika na inny. Oczywiście istnieje lepsze rozwiązanie:

   1:  public interface BetterIButtonClient
   2:  {
   3:      void Switch();
   4:  }
   5:   
   6:  public interface BetterIButton
   7:  {
   8:      void Click();
   9:  }
  10:   
  11:  public class BetterButton : BetterIButton
  12:  {
  13:      BetterIButtonClient client;
  14:      public BetterButton(BetterIButtonClient aClient)
  15:      {
  16:          client = aClient;
  17:      }
  18:   
  19:      public void Click()
  20:      {
  21:          this.client.Switch();
  22:      }
  23:  }
  24:   
  25:  public class FancyBetterButton : BetterIButton
  26:  {
  27:      BetterIButtonClient client;
  28:      public FancyBetterButton(BetterIButtonClient aClient)
  29:      {
  30:          client = aClient;
  31:      }
  32:   
  33:      public void Click()
  34:      {
  35:          this.client.Switch();
  36:      }
  37:  }
  38:   
  39:  public class BetterLamp : BetterIButtonClient
  40:  {
  41:      BetterIButton betterButton;
  42:      public BetterLamp(BetterIButton aBetterButton)
  43:      {
  44:          betterButton = aBetterButton;
  45:      }
  46:   
  47:      public void Switch()
  48:      {
  49:          // switch the light on/off
  50:      }
  51:  }

Co dostajemy? Otóż button już nie musi działać tylko i wyłącznie z lampą, zadziała także z pralką, lodówką i innym AGD o ile sprzęt będzie obsługiwać interfejs BetterIButtonClient i prostą metodą Switch. A dalej widać, że lampa także może być już bardziej fikuśna, nie zależy ona już tylko od jednego rodzaju Buttona. W dodatku to nie ona jest odpowiedzialna za decydowanie z którego skorzysta (od siebie dodałem idee: DI). Dzięki temu fabryka lamp może w prosty sposób przerzucić się na nowszy model z wykorzystaniem FancyBetterButton-ów. Widać tu wpływy idei OCP. Taki kod prościej jest wykorzytać w innym projektach, ponieważ nie jest on mocno powiązany z bezpośrednią implementacją klasy guzików czy innych obiektów, zdefiniowanych z projekcie.

Podczas pisania kodu należy przede wszystkim korzystać z G.Ł.O.W.Y. to jest najważniejsza zasada! Dopiero później warto podpierać się różnymi ideami i regułami, które proponują inni. Zasady są po to aby je łamać, a nic tak nie uczy człowieka jak popełniane błędy. Zachęcam do łamania zasad SOLID, a potem sprawdzenia czy miało to sens i podzielenia się tym doświadczeniem ze mną i innymi 😉

Podczas wpisu sporą część (bardzo dużą część wiedzy) oparłem na
[1] http://www.blackwasp.co.uk/SOLIDPrinciples.aspx
[2] www.wikipedia.org polska i angielska wersja
[3] http://www.objectmentor.com Tutaj trzeba się porządnie naszukać aby znaleźć materiały
Szczególnie polecam punkt pierwszy.

Uwagi jak zawsze chętnie przyjmę.

ps. Jutro do pracy
ps2. C# jest czytelny, nawet dla nie programujących w C#.
ps3. Link do źródełek ze wszystkimi zasadami  https://www.sugarsync.com/pf/D6056980_3239101_969599
 

Dlaczego warto rozmawiać (o kodzie)

Uhu, dawno nie pisałem. Już już,  nie płaczcie.
Dlaczego warto rozmawiać, pytać, poprawiać i pokazać komuś swój kod? Najtrudniej jest znaleźć swoje błędy, trudno jest spojrzeć na swój kod i zapytać się “czemu zrobiłem to tak – a nie w inny sposób?”. Skoro wszystko działa tak jak założyłem na początku to znaczy, że jest dobrze i refaktoryzacja jest nie potrzebna. Jeśli się uruchamia, działa i nie wywala to po co mi napisać testy. I w końcu skoro się tyle namęczyłem na tym wszystkich, użyłem całej swojej wiedzy, internetu i haków gdzieś znalezionych to czemu mam to komuś pokazać? To moja wiedza tajemna, nie chce się tym dzielić, nie chce żeby ktoś zobaczył mój workaround na jakiś problem.

Testy, idea TDD pewnie każdy słyszał i mówił że super i fajnie, że trzeba i co? I życie, nie chce się pisać. Jak też nie pisałem testów, bo to projekt gdzie się będę uczyć nowego, że więcej czasu spędzę nad testami niż napisaniem właściwego kodu. Teraz siedzę sobie na testami i wychodzą różne kwiatki, które napisałem na szybko, na chwilę, żeby się skompilowało. Można powiedzieć że robię review samemu sobie. Taki przykładzik:

.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: Consolas, “Courier New”, Courier, Monospace;
background-color: #ffffff;
max-height: 300px;
overflow: auto;
/*white-space: pre;*/
}

.csharpcode pre { margin: 0em; }

.csharpcode .rem { color: #008000; }

.csharpcode .kwrd { color: #0000ff; }

.csharpcode .str { color: #a31515; }

.csharpcode .op { color: #0000c0; }

.csharpcode .preproc { color: #cc6633; }

.csharpcode .asp { background-color: #ffff00; }

.csharpcode .html { color: #800000; }

.csharpcode .attr { color: #ff0000; }

.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}

.csharpcode .lnum { color: #606060; }

   1:  namespace CompactCal.Model
   2:  {
   3:      using System;
   4:      using System.Collections.Generic;
   5:      using Microsoft.Phone.UserData;
   6:   
   7:      public class CalendarEntry
   8:      {
   9:          public CalendarEntry(Appointment appointment)
  10:              : this(
  11:                  appointment.Attendees,
  12:                  appointment.Details,
  13:                  appointment.StartTime,
  14:                  appointment.EndTime,
  15:                  appointment.IsAllDayEvent,
  16:                  appointment.Location,
  17:                  appointment.Organizer,
  18:                  appointment.Subject)
  19:          {
  20:          }
  21:   
  22:          public CalendarEntry()
  23:          {
  24:              
  25:          }
  26:   
  27:          public CalendarEntry(
  28:              IEnumerable<Attendee> attendees,
  29:              string details,
  30:              DateTime startTime,
  31:              DateTime endTime,
  32:              bool isAllDay,
  33:              string location,
  34:              Attendee organizer,
  35:              string subject)
  36:          {
  37:              this.Attendees = new List<CalendarPerson>();
  38:              foreach (var a in attendees)
  39:              {
  40:                  this.Attendees.Add(new CalendarPerson(a));
  41:              }
  42:              this.Details = details;
  43:              this.StartTime = startTime;
  44:              this.EndTime = endTime;
  45:              this.IsAllDayEvent = isAllDay;
  46:              this.Location = location;
  47:              if (organizer != null)
  48:              {
  49:                  this.Organizer = new CalendarPerson(organizer);
  50:              }
  51:              this.Subject = subject;
  52:   
  53:              this.Duration = this.EndTime.Subtract(this.StartTime);
  54:          }
  55:   
  56:          public CalendarEntry(
  57:              IEnumerable<CalendarPerson> attendees,
  58:              string details,
  59:              DateTime startTime,
  60:              DateTime endTime,
  61:              bool isAllDay,
  62:              string location,
  63:              CalendarPerson organizer,
  64:              string subject)
  65:          {
  66:              this.Attendees = new List<CalendarPerson>();
  67:              this.Attendees.AddRange(attendees);
  68:              this.Details = details;
  69:              this.StartTime = startTime;
  70:              this.EndTime = endTime;
  71:              this.IsAllDayEvent = isAllDay;
  72:              this.Location = location;
  73:              this.Organizer = organizer;
  74:              this.Subject = subject;
  75:   
  76:              this.Duration = this.EndTime.Subtract(this.StartTime);
  77:          }
  78:   
  79:          public List<CalendarPerson> Attendees { get; private set; }
  80:   
  81:          public string Details { get; private set; }
  82:   
  83:          public DateTime StartTime { get; private set; }
  84:   
  85:          public DateTime EndTime { get; private set; }
  86:   
  87:          public bool IsAllDayEvent { get; private set; }
  88:   
  89:          public string Location { get; private set; }
  90:   
  91:          public CalendarPerson Organizer { get; private set; }
  92:   
  93:          public string Subject { get; private set; }
  94:   
  95:          public TimeSpan Duration { get; private set; }
  96:      }
  97:  }

I co widać, syfek. Kod dla konstruktorów się powtarzają, w ogóle jest ich za dużo. Same dziwy ;). Czemu tak? Bo mi się śpieszyło żeby rozwiązać jakiś problem i tak zostawiłem, a skoro działało to nie wracałem do tematu. Dopiero gdy zabrałem się za testowanie poprawiłem kod na taki który mnie bardziej zadowala:

   1:  using System;
   2:  using System.Collections.Generic;
   3:  using System.Linq;
   4:  using Microsoft.Phone.UserData;
   5:   
   6:  namespace Model
   7:  {
   8:      public class CalendarEntry
   9:      {
  10:          public CalendarEntry(Appointment appointment) : this(
  11:          appointment.Attendees.ConvertToCalendarPersons(),
  12:          appointment.Details,
  13:          appointment.StartTime,
  14:          appointment.EndTime,
  15:          appointment.IsAllDayEvent,
  16:          appointment.Location,
  17:          appointment.Organizer.ConvertToCalendarPerson(),
  18:          appointment.Subject)
  19:          {
  20:          }
  21:   
  22:          public CalendarEntry(
  23:              IEnumerable<CalendarPerson> attendees,
  24:              string details,
  25:              DateTime startTime,
  26:              DateTime endTime,
  27:              bool isAllDay,
  28:              string location,
  29:              CalendarPerson organizer,
  30:              string subject)
  31:          {
  32:              Attendees = new List<CalendarPerson>();
  33:              Attendees.AddRange(attendees);
  34:              Details = details;
  35:              StartTime = startTime;
  36:              EndTime = endTime;
  37:              IsAllDayEvent = isAllDay;
  38:              Location = location;
  39:              Organizer = organizer;
  40:              Subject = subject;
  41:   
  42:              Duration = EndTime.Subtract(StartTime);
  43:          }
  44:   
  45:          public List<CalendarPerson> Attendees { get; private set; }
  46:   
  47:          public string Details { get; private set; }
  48:   
  49:          public TimeSpan Duration { get; private set; }
  50:   
  51:          public DateTime EndTime { get; private set; }
  52:   
  53:          public bool IsAllDayEvent { get; private set; }
  54:   
  55:          public string Location { get; private set; }
  56:   
  57:          public CalendarPerson Organizer { get; private set; }
  58:   
  59:          public DateTime StartTime { get; private set; }
  60:   
  61:          public string Subject { get; private set; }
  62:      }
  63:  }

Co się zmieniło? Wywaliłem dwa konstruktory, jeden z nich okazał się w ogóle nie używany. A drugi był nadmiarowy. Kod jest krótszy i się nie powtarza. Prostszy do utrzymania i ewentualnych poprawek czy zmian. Wszystko dzięki review, napisaniu kilku linijek kodu testowego, oraz chwili przemyślenia. Myślę że gdyby ten kod zobaczył ktoś inny, wcześniej, zanim wybrałem opcję commit nie doszło bo takiej sytuacji, gdzie zmiany byłyby potrzebne. Najprawdopodobniej musiałbym wcześniej to poprawić zanim będę mógł wrzucić zmiany na serwer. Ale pracuje sam, gdzie nie ma mnie kto opieprzyć. Review kodu to naprawdę dobra, szybka i tania możliwość znalezienia prostych błędów. Jeśli tylko jest taka możliwość, aby ktoś przejrzał nasz kod, należy z tego skorzystać.

Opowiadałem kilku znajomym co zrobiłem, jak to zrobiłem, i że w ogóle takie fajne. Padło stwierdzenie że niezły hack, ale nie jest to do końca czyste zagranie. Chodzi o deklarację interfejsu, który będzie posiadać event. Ale od początku: napisałem własnego delegata, później event, F7, build i błąd: interfejs nie może definiować własnych typów. Pierwszy odruch google “how to declare event in interface” – pierwszy wynik MSDN, wszystko pięknie, CTRL+C/V – super, działa, walczę dalej.

   1:  namespace ImplementInterfaceEvents
   2:  {
   3:      public interface IDrawingObject
   4:      {
   5:          event EventHandler ShapeChanged;
   6:      }
   7:      public class MyEventArgs : EventArgs 
   8:      {
   9:          // class members
  10:      }
  11:      public class Shape : IDrawingObject
  12:      {
  13:          public event EventHandler ShapeChanged;
  14:          public void ChangeShape()
  15:          {
  16:              // Do something here before the event…
  17:              OnShapeChanged(new MyEventArgs(/*arguments*/));
  18:              // or do something here after the event. 
  19:          }
  20:          protected virtual void OnShapeChanged(MyEventArgs e)
  21:          {
  22:              if(ShapeChanged != null)
  23:              {
  24:                 ShapeChanged(this, e);
  25:              }
  26:          }
  27:      }
  28:   
  29:  }

To kod z MSDN, potem takiego eventa w obsłudze trzeba rzutować na MyEventArgs jeśli chce się odczytać dodatkowe właściwości z argumentu. Nawet się nie zastanowiłem, że mogę coś zrobić inaczej, skoro MSDN tak pokazuje to powinno być dobrze. Właśnie do tego rozwiązania padł zarzut, że to niezły trik/hack, ale nie jest czysty. Że lepiej jest zdefiniować własny typ dla delegata i z niego korzystać w interfejsie. Jak – skoro wywala błąd (czasem jestem taki głupiutki)?!.

   1:  using System;
   2:   
   3:  namespace GeocodeService
   4:  {
   5:      public delegate void QueryDelegate(object sender, GeoLocation location);
   6:   
   7:      public interface IGeocode
   8:      {
   9:          void Query(string aLocation);
  10:          event QueryDelegate QueryCompleted;
  11:      }
  12:   
  13:  }

Ech 🙂 Teraz nie trzeba nic rzutować, wszystko jest proste i czyste. Pewnie gdybym nie pokazał tego swojego cuda, to został bym z przykładem z msdn i byłbym z niego zadowolony. Żyłbym sobie w błogiej nieświadomości.

Jeśli piszesz solo staraj się używać TDD, to pierwszy front kontroli twojej jakości kodu który tworzysz. Uważasz że TDD to strata czasu, napisz od czasu do czasu prosty test, sprawdź czy to napisałeś wtedy, nadal jest potrzebne teraz i czy spełnia wszystkie założenia.
Poproś kogoś aby zerknął na kawałek kodu, jeśli nie zrozumie tego co widzi to znaczy że może jest to przekombinowane, może da się uprościć. Przeczytaj czasem kod kolegi, zapytaj się czemu napisał to tak a nie inaczej, może Ty dowiesz się czegoś nowego, lub on zastanowi się nad tym co napisał i to poprawi. O kodzie, o tym jak jest napisany i pomyślany naprawdę rozmawiać.

Co o tym sądzicie? Mam rację? Gadam głupoty?