czwartek, stycznia 02, 2014

Refactoring bez testów to proszenie się o problemy ;-)

W Bloggeroidzie mam sobie klasę, która służy do przechowywania danych o blogu.
Klasa nazywa się Blog.
Ma 4 pola i settery oraz getter... znaczy miała, teraz już nie ma setterów.
Bo czytając ponownie Effective Java postanowiłem przerobić Bloga w klasę, którą buduje się Builderem.

I wszystko OK, ale mam też klasę BloggerWrapper, która gada z Bloggerem.

Jest tam sobie mały parser SAXowy, który parsuje odpowiedź Bloggera. Do niedawna wyglądała metoda endElement tak:


 public void endElement(String uri, String localName, String qName) throws SAXException {
if (localName.equals("id") && blog!=null) {
blog.setBlogId(sb.toString());
} else if (localName.equals("title") && blog!=null) {
blog.setBlogName(sb.toString());
} else if (localName.equals("entry") && blog!=null) {
list.add(blog);
blog=null;
}
super.endElement(uri, localName, qName);
}

Ale po wprowadzeniu Buildera już blog nie ma setterów więc kod zmienił się w:


public void endElement(String uri, String localName, String qName) throws SAXException {
if (localName.equals("id") && blog!=null) {
blogBuilder.setBlogId(sb.toString());
} else if (localName.equals("title") && blog!=null) {
blogBuilder.setBlogName(sb.toString());
} else if (localName.equals("entry") && blog!=null) {
list.add(blogBuilder.build());
blogBuilder=null;
}
super.endElement(uri, localName, qName);
}
Wszystko cacy....
Jednak tak się składa, że blog to też nazwa zmiennej w metodzie owijającej klasę anonimową parsera....
Więc blog jest dostępny w tym kodzie i tak się składa, że jego wartość to null....
Więc cała banda ifów nigdy się nie wykonuje....

Poprawny kod, w miejsce blog!=null ma blogBuilder!=null.

Chciałem sobie porefactorować, żeby kod był ładniejszy, ale testów nie mam (do tego akurat testy też nie są najprostsze, chociaż można je zrobić) no i dostałem po zębach ;-)
Dawno tak fajnego błędu nie zrobiłem ;-)

Gdyby nie czujny użytkownik Bloggeroida to mógłbym tego nie zauważyć przez najbliższe parę tygodni i dziwiłbym się czemu dostaję słabsze oceny....


Podobne postybeta
Dalsze walki Androidowe
Niecne wykorzystanie refleksji... czyli jak poszukać tekstu w drzewie obiektów? ;-)
Jak zostać wrogiem wolności słowa ;-)
Umiejętność programowania pomaga :-)
assertEquals > assertThat ;-)