Nadmiar boli, szczególnie jeżeli dotyczy to instrukcji warunkowych. Mamy metodę w której roi się od if-ów, cóż mam czynić z  tym kodem, kontynuować?
Dużo instrukcji warunkowych sprawia, że kod jest nie czytelny, dodatkowo jeżeli jest wiele poziomów zagnieżdżenia cztery,pięć i więcej a nawet trzy to za dużo bardzo łatwo o błąd przy jakiejkolwiek zmianie, a i bez zmiany to nie można mieć pewności że taki kod działa poprawnie.
Przy okazji refaktoryzacji, natknąłem się na kod w którym jest kilka if-ów wielokrotnie zagnieżdżonych (wielokrotnie przełożonych), cóż kod ten pochodzi z końca 2017 roku gdzie moja świadomość dotycząca pisania czystego, utrzymywanego kodu była znikoma. Projekt wrócił jak bumerang, a potrzeba wprowadzić pewne zmiany. Co w kodzie źle napisanym jest trudne. Z tego co piszą testowanie kodu z warunkami też nie jest proste, na razie mam problemy z testami więc nie mam wyrobionego zdania. Poniżej przedstawiłem kilka rozwiązań które przychodzą mi na myśl aczkolwiek, ja napotykając na if-y staram się je redukować, jeśli to możliwe usuwać, a jeżeli jest to niewykonalne tudzież zbyt trudne (za dużo warunków, za dużo punktów do zmian) to w ostateczności można je „wykopać” do metod i tam sterowanie przenieść, dzięki temu przynajmniej zredukujemy kod w np kontrolerze. Według ankel Boba nie powinno być więcej niż jedno zagnieżdżenia instrukcji warunkowej.
Zastanawiałem się czy możliwe jest w ogóle zredukowanie warunków, czy da się napisać program tak żeby przyjmował parametry początkowe, przeszedł przez pewien ciąg instrukcji i wypluł wyniki , a wszystko jak by w jednym ciągu? Po przy długim wstępie rozwiązania na tę chwilę.
Wyjść jest kilka, może na początek wymienię możliwości, które ja widzę:

  • można kontynuować koncepcję pisać kolejnego if-a
  • spróbować z redukować if-y, wyrzucając choćby instrukcję do metod, w których część warunków umieścimy
  • można się pokusić o wzorzec strategi, który w założeniu pomoże nam zredukować instrukcje warunkowe

Mamy przykładowy kod części aplikacji do obsługi zleceń kupna sprzedaży towarów (bliżej nieokreślonych). Ta część prezentowanego kodu, była posadzona w akcji kontrolera, wyciągnąłem go do metody. Ważna uwaga nigdy, przenigdy nie pakuj wszystkiego do kontrolera, nauczyłem się, że kontroler powinien być jak najlżejszy. Najlepiej całą logikę trzymać w osobnych klasach, w kontrolerze tylko je wywoływać. Ja na początku mojej przygody z MVC zapomniałem, że kontroler to klasa z propertasami i metodami. Teraz gdy robię refktoryzację tego kodu widzę jak wielki błąd popełniłem (;-). Kolejna uwaga do wzorca MVC , kontroler jest tylko cienką warstwą między widokiem, a modelem. Pod słowem model kryje się nie tylko klasy wygenerowane w EF ale cała logika, warstwa abstrakcji, czy repozytorium, które łączy się z resztą aplikacji za pomocą interfejsów.

Ten kod wybrałem bo nie jest za duży, prezentuje jak nie powinno się pisać warunków i jak wyjść z „twarzą” z takiej sytuacji. W mojej aplikacji znajduje się o wiele więcej takich smaczków (smrodków).

Opis kodu.
Mam tutaj metodę która ma zwracać listę zleceń kupna, sprzedaży. Typem dla listy jest widok vZlecenieKupnoSprzedazTowar, zdefiniowny w bazie i pociągnięty przez EF-a, ja uważam że lepszy do tworzenie widoków jest MS SQL (szczególnie jak się robi w podejściu Base First). Konkretne pola w tym momencie nie mają znaczenia ani ich typy. Pierwszą rzeczą którą można tu wykonać, a która również jest refaktoryzacja to zmiana nazwy metody na coś więcej niż getList bo to naprawdę nic nie mówi. Można by tu było dać komentarz, kiedyś tak robiłem dawałem komentarze wszędzie i dużo bo myślałem że to dobre ale tak nie jest, Komentarze kłamią.
Kolejna sprawa to używanie kontekstu z EF-a to używał bym tylko w repozytorium, a jeżeli potrzebował bym zapytać o jakieś dane napisał bym odpowiednią metodę i poprzez interfejs wypuścił ją na zewnątrz, wiem w ten sposób robimy sobie dodatkową robotę, interfejs, klasa, wrzucenie do kontenera zależności (jeśli używamy).
Kolejną rzeczą, która rzuca się w oczy jest zawartość każdego if-a, to jest to samo tylko ma inny parametr wejściowy jeden bo idZlec jest takie samo dla całej metody.

public List<vZlecenieKupnoSprzedazTowar> getList(int idZlec, string statusZlec)
{
using (var context = new Models.Entities())
{
int status = 0;
var wynik = context.vZlecenieKupnoSprzedazTowar.AsNoTracking()
.Where(x => x.id == idZlec && x.status == statusZlec)
.AsEnumerable().ToList();
if (wynik.Any())
{
if (wynik.Where(x => x.id == idZlec && x.status == „Zakończone”).Any())
{
wynik = wynik.Where(x => x.id == idZlec && x.status == „Zakończone”)
.Distinct().ToList();
status = 3;
ViewBag.status = „Zakończone”;
Session[„statusZlecenia”] = „Zakończone”;
}
if (wynik.Where(x => x.id == idZlec && x.status == „W realizacji”)
.Any() && status < 3)
{
wynik = wynik.Where(x => x.id == idZlec && x.status == „W realizacji”)
.Distinct().ToList();
status = 2;
ViewBag.status = „W realizacji”;
Session[„statusZlecenia”] = „W realizacji”;
}
if (wynik.Where(x => x.id == idZlec && x.status == „Nowe”)
.Any() && status < 2)
{
wynik = wynik.Where(x => x.id == idZlec && x.status == „Nowe”)
.Distinct().ToList();
status = 1;
ViewBag.status = „Nowe”;
Session[„statusZlecenia”] = „Nowe”;
}
}
return wynik;
}
}

Kod po redukcji
Jakże prosty, a przecież to nie koniec,. Mamy nową nazwę, mamy tylko jednego if-a kod jest czytelniejszy.
Komentarz do metody nie jest potrzebny bo wszystko jest w nazwie która mówi co jest zwracane i jakie parametry są oczekiwane oraz że jest to pobranie a nie zapis. Sprawą dyskusyjną jest to czy nazwa jest za długa, ja uważam że nie gdyż patrząc na taką nazwę wiem czego się spodziewać, owszem należy nie przesadzić.

public List<vZlecenieKupnoSprzedazTowar>
getListOfElementsVZlecenieElementZamowienieBy_idZlec_and_status(int idZlec, string statusZlec)
{
using (var context = new Models.Entities())
{
List result = context.vZlecenieKupnoSprzedazTowar.AsNoTracking()
.Where(x => x.id == idZlec && x.status == statusZlec)
.AsEnumerable().Distinct().ToList();
if (result.Any())
{
ViewBag.status = statusZlec;
Session[„statusZlecenia”] = statusZlec;
}
return result;
}
}

Kolejna redukcja
Usunąłem kontekst z EF-a, zamiast odwoływać się bez pośrednio poprzez kontekst i pytać o dane zrobiłem to za pomocą interfejsu i metody getListOfvZlecenieKupnoSprzedazTowarBy_idZlec_and_status z interfejsu _iVvZlecenieKupnoSprzedazTowar. Właściwie po takiej redukcji można by się pokusić o wyrzucenie metody i wprowadzenie kodu z powrotem do kontrolera, kwestie co zrobić ze zmienną statusZlec na razie pozostawiam otwartą (nic mi nie przychodzi do głowy).

public List<vZlecenieKupnoSprzedazTowar> getvZlecenieKupnoSprzedazTowarBy_idZlec_and_status(int idZlec, string statusZlec)
{
var result = _ivZlecenieKupnoSprzedazTowar
.getListOfvZlecenieKupnoSprzedazTowarBy_idZlec_and_status(idZlec,statusZlec);
if (result.Any())
{
ViewBag.status = statusZlec;
Session[„statusZlecenia”] = statusZlec;
}
return result;
}