Projekt pokazał mi, że jakość kodu jest ważna!

Wstęp.

Panel logowania

Od ostatniego wpisu minął ponad miesiąc, ale mniejsza z tym. W tym wpisie mówiłem o realizowaniu projektu w Javie na studia. Niestety z racji skróconego (a zarazem skondensowanego) semestru wykańczanie tego projektu to była nerwówka. Wyszło to średnio. Postanowiłem podzielić się przemyśleniami, które naszły mnie w trakcie ostatniego tygodnia prac nad projektem.

Panel administracyjny

Gwoli przypomnienia - w skład projektu wchodzi aplikacja kliencka napisana w Swingu, która umożliwia użytkownikom o różnym stopniu dostępu, rozwiązywanie testów wielokrotnego wyboru, podgląd wyników testów, zarządzanie testami, użytkownikami itp. Aplikacja serwera przyjmuje połączenia od klientów i wykonuje zapytania od nich płynące, przechowuje testy, użytkowników i inne potrzebne dane w bazie danych, z którą się komunikuje przy użyciu JDBC. 

Nie zawsze to, co wydaje się być dobre w fazie projektowania, okazuje się być praktyczne.

Podczas modelowania dziedziny założyłem sobie, że aby zapewnić sobie rozszerzalność programu przyda mi się polimorfizm. Zrobiłem sobie poglądowy diagram klas (ale nie jest to UML), żeby nie iść na żywioł z pisaniem kodu.

Model dziedziny

Mamy jakąś tam abstrakcyjną klasę Zapytanie, każdy rodzaj zapytania implementuje metodę execute(), która wykonuje to, co należy. Przesyłamy konkretne Zapytanie z potrzebnymi danymi do serwera, który wywołuje metodę execute() i pakuje wyniki do Zapytania i odsyła je do klienta. Tak samo działa ZapytanieSQL, tyle, że dotyczy komunikacji serwer-baza danych. Sama idea wydaje mi się być bardzo dobra: możemy dodawać nowe zapytania nie modyfikując kodu odpowiedzialnego za ich obsługę.  Żeby można było sobie wyobrazić to, co wyżej napisałem załączam po przykładzie:

// >> ZapytanieSQL <<
public class SQLInsertGroup implements SQLStatement {
 public SQLInsertGroup(int number, String specialization, String academicYear){
  this.number = number; 
  this.specialization = specialization; 
  this.academicYear = academicYear;
 }
 
 @Override
 public ResultSet execute(Connection dbConnection) throws SQLException {
  String query = "INSERT INTO studentGroup VALUES(default, " + 
  number + ",'" + specialization + "', '" + academicYear + "');";
  Statement stmt = null;
  
  stmt = dbConnection.createStatement();
  stmt.executeUpdate(query);
  
  if(stmt != null) stmt.close();

  return null;
 }
 
 private int number;
 private String specialization;
 private String academicYear;
}

// >> Zapytanie <<
public class NetAddGroup extends NetStatement {
 public NetAddGroup(int number, String specialization, String academicYear) {
  this.number = number;
  this.specialization = specialization;
  this.academicYear = academicYear;
 }
 
 @Override
 public void execute(DBManager dbMgr, AvailableTestsManager testMgr) {
  try {
   ResultSet rs = dbMgr.executeSQLStatement(
            new SQLInsertGroup(number, specialization, academicYear));
  } catch (SQLException e) {
   errorFlag = true;
   errorMsg = e.getMessage();
  }
 }
 
 /** [in] Numer grupy. */
 private int number;
 /** [in] Specjalizacja. */
 private String specialization;
 /** [in] Rok akademicki np. 2011/2012. */
 private String academicYear;
}

Co poszło nie tak?

Klasy, klasy, klasy...

Więc co tutaj najbardziej dawało się we znaki i wymagało by poprawy przy refactoringu? Okazało się, że wszystkie podklasy ZapytaniaSQL (SQLStatement) różniły się z reguły tylko polami, konstruktorem, który robił to samo, tylko że z innymi nazwami argumentów i pól. Usprawiedliwiając się goniącym terminem brnąłem w ten obłęd - kopiowanie jakiejś istniejącej klasy, podmiana nazw i Stringa query z treścią zapytania zapisanego w języku SQL. Takim sposobem miałem bardzo długą listę klas SQLGetCośTam, SQLInsertCośTam, parę SQLUpdateCośTam i SQLDeleteCośTam, które powstawały poprzez schemat kopiuj-zmień w paru miejscach. Myślę, że tutaj zastosowanie mechanizmu polimorfizmu to przesada.

O ile zapytania klient-serwer zrealizowane w takiej postaci są dobre, to i tam wkradło się nieprzemyślane działanie. Tworzyłem dla większości ZapytańSQL odpowiednik zapytania klient-serwer. Jak widać, pola klas się w tym przypadku pokrywają. Jeżeli były to zapytania pobierające dane, to dodatkowo dochodziły w klasie pochodnej po NetStatement dodatkowe pola na wyniki. W zasadzie tworzył on tylko to ZapytanieSQL wykonywał je, wyciągał z obiektu ResultSet potrzebne dane wpisując je do pól składowych klasy. Masa takiego samego kodu. Jednym prostym rozwiązaniem ograniczającym kopiowanie było by po prostu zastąpienie tych 3 składowych klasy NetAddGroup jedną typu SQLStatement. Myślę, że lepiej było by ograniczyć ilość zapytań tego typu poprzez wprowadzenie ogólniejszych: NetSendDataToDatabase, NetGetDataFromDatabase które zwracało by do klienta ResultSet czy inny ogólny pojemnik na te dane. Jedno jest pewne, coś tutaj należało by poprawić.


Podsumowując.

Miał to być taki wpis z moimi przemyśleniami, nie chcę się wdawać w szczegóły. Chciałem tylko powiedzieć, że

• nie zawsze to, co wydaje się być dobre, okazuje się wygodne w użyciu
• trzeba przemyśleć coś dwa razy zanim zacznie się klepać kod
• jeżeli w trakcie pisania kodu zauważymy, że coś można poprawić, to zróbmy to! Odkładanie tego na później tylko pogarsza sprawę.
• goniący termin potrafi skutecznie zachęcać nas do pisania złego kodu, bo przecież nie ma czasu, ale mimo wszystko, szczególnie jeśli projekt będzie rozwijany w przyszłości - trzeba się pilnować.

Wyciągnąłem wnioski z tego projektu - mimo, że kod wymaga sporych poprawek i na tym etapie wstydzę się go, to zdałem sobie sprawę z tego, że utrzymywanie dobrej jakości pisanego kodu jest ważne. Skłoniło mnie to do rozpoczęcia lektury książki Czysty kod. Podręcznik dobrego programisty Roberta C. Martina. Życzę wszystkim programistom jak najlepszego kodu wypływającego spod ich klawiatury.