Czasami trafi się naprawdę ciekawy przypadek refaktoryzacji kodu. Jeżeli ma się szczęście to będzie to przypadek na tyle krótki, że cały proces refaktoryzacji można przeprowadzić w 15 minut wraz ze zgłoszeniem propozycji zmiany do właściciela kodu.
Stefan Koopmanschap, w trakcie 4Developers opowiadał o refaktoryzacji kodu. Podzielił on ten proces na dwa. Pierwszy to refaktoryzacja drugi to przepisanie. W pierwszym przypadku nie naruszamy publicznego API modułu w drugim pojawiają się zmiany. W moim przypadku mamy do czynienia z sytuacją pośrednią. Kod wykorzystywany w projekcie nie uległ zmianie, ale zmianie uległo publiczne API. Jak to możliwe?

Specyficzna klasa

Czasami zdarza się, że tworzymy klasy, których jedynym zadaniem jest przetrzymywanie dużych zbiorów niezmiennych danych. Do javy 1.4 robiło się to za pomocą pól statycznych-finalnych odpowiedniego typu. Od czasu javy 1.5 mamy enumy. Omawiany przypadek to właśnie zamiana klasy z takimi polami na enum.
Oryginalny kod klasy wygląda następująco:

Listing 1. Klasa ze stałymi. Nie rób tak

/**
 * Language.java
 *
 * Copyright (C) 2007,  Richard Midwinter
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program. if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 */
package com.google.api.translate;

import java.util.Arrays;
import java.util.List;

/**
 * Defines language information for the Google Translate API.
 * 
 * @author Richard Midwinter
 * @author alosii
 */

public final class Language {
	public static final String AUTO_DETECT = "";
	public static final String ARABIC = "ar";
	public static final String BULGARIAN = "bg";
	public static final String CATALAN = "ca";
	public static final String CHINESE = "zh";
	public static final String CHINESE_SIMPLIFIED = "zh-CN";
	public static final String CHINESE_TRADITIONAL = "zh-TW";
	public static final String CROATIAN = "hr";
	public static final String CZECH = "cs";
	public static final String DANISH = "da";
	public static final String DUTCH = "nl";
	public static final String ENGLISH = "en";
	public static final String FILIPINO = "tl";
	public static final String FINNISH = "fi";
	public static final String FRENCH = "fr";
	public static final String GALACIAN = "gl";
	public static final String GERMAN = "de";
	public static final String GREEK = "el";
	public static final String HEBREW = "iw";
	public static final String HINDI = "hi";
	public static final String HUNGARIAN = "hu";
	public static final String INDONESIAN = "id";
	public static final String ITALIAN = "it";
	public static final String JAPANESE = "ja";
	public static final String KOREAN = "ko";
	public static final String LATVIAN = "lv";
	public static final String LITHUANIAN = "lt";
	public static final String MALTESE = "mt";
	public static final String NORWEGIAN = "no";
	public static final String POLISH = "pl";
	public static final String PORTUGESE = "pt";
	public static final String ROMANIAN = "ro";
	public static final String RUSSIAN = "ru";
	public static final String SERBIAN = "sr";
	public static final String SLOVAK = "sk";
	public static final String SLOVENIAN = "sl";
	public static final String SPANISH = "es";
	public static final String SWEDISH = "sv";
	public static final String THAI = "th";
	public static final String TURKISH = "tr";
	public static final String UKRANIAN = "uk";
	public static final String VIETNAMESE = "vi";
	
	public static final List<String> validLanguages = Arrays.asList(new String[] {
			AUTO_DETECT, ARABIC, BULGARIAN, CATALAN, CHINESE, CHINESE_SIMPLIFIED,
			CHINESE_TRADITIONAL, CROATIAN, CZECH, DANISH, DUTCH,
			ENGLISH, FILIPINO, FINNISH, FRENCH, GALACIAN, GERMAN,
			GREEK, HEBREW, HINDI, HUNGARIAN, INDONESIAN, ITALIAN,
			JAPANESE, KOREAN, LATVIAN, LITHUANIAN, MALTESE, NORWEGIAN,
			POLISH, PORTUGESE, ROMANIAN, RUSSIAN, SERBIAN, SLOVAK,
			SLOVENIAN, SPANISH, SWEDISH, THAI, TURKISH, UKRANIAN, VIETNAMESE
	});
	
	/**
	 * Checks a given language is available to use with Google Translate.
	 * 
	 * @param language The language code to check for.
	 * @return true if this language is available to use with Google Translate, false otherwise.
	 */
	public static boolean isValidLanguage(String language) {
		return validLanguages.contains(language);
	}
}

źródło

Kod pochodzi z projektu google-api-translate-java, a klasa reprezentuje listę języków obsługiwanych w tłumaczu googla. Kod składa się z trzech sekcji. Pierwsza to spis języków reprezentowany przez pola klasy. Druga to statyczna lista języków. Trzecia to metoda sprawdzająca czy dany język jest obsługiwany. Jak nie trudno zauważyć mamy tu ciekawy przypadek błędu. Klasa przechowuje listę wartości w dwóch miejscach jako oddzielne pola i jako listę. Dodatkowo jest metoda sprawdzająca czy dany język jest obsługiwany. Sprawdzanie odbywa się za pomocą wyszukiwania wartości w liście. Już tutaj można się pokusić o stwierdzenie, że kod będzie trudny w refaktoryzacji. Ja stwierdzam, że jest to ciekawy przypadek.

Pierwszy krok, szukanie odwołań.

Zanim przystąpiłem do niszczenia kodu sprawdziłem gdzie można spodziewać się problemów. Największym problemem mogła okazać się gęsto występująca metoda isValidLanguage ponieważ musiałbym zmienić cały kod za jednym podejściem (błędy kompilacji brak metody!) lub jechać z jakąś protezą. Na całe szczęście metoda była użyta raz, ale widowiskowo:

Listing 2. Nie róbcie tak. NIGDY!

private static String retrieveTranslation(String text, String from, String to) throws Exception {
        if (!Language.isValidLanguage(from) || !Language.isValidLanguage(to) || Language.AUTO_DETECT.equals(to)) {
                throw new IllegalArgumentException("You must use a valid language code to translate to and from.");
        }
        //....
}

Pytanie co jest to źle? Oczywiście wszystko 😀 Zresztą ten kawałek utwierdził mnie w przekonaniu, że należy refaktoryzować ten kod.

Zmiany

Co tu się rozpisywać. Oto nowa wersja klasy, teraz już enuma.

Listing 3. Brzydkie kaczątko zamienia się w pięknego enuma

/**
 * Language.java
 *
 * Copyright (C) 2007,  Richard Midwinter
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program. if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 */
package com.google.api.translate;

/**
 * Defines language information for the Google Translate API.
 * 
 * @author Richard Midwinter
 * @author alosii
 */

public enum Language {

	AUTO_DETECT(""), ARABIC("ar"), BULGARIAN("bg"), CATALAN("ca"), CHINESE("zh"), CHINESE_SIMPLIFIED(
			"zh-CN"), CHINESE_TRADITIONAL("zh-TW"), CROATIAN("hr"), CZECH("cs"), DANISH(
			"da"), DUTCH("nl"), ENGLISH("en"), FILIPINO("tl"), FINNISH("fi"), FRENCH(
			"fr"), GALACIAN("gl"), GERMAN("de"), GREEK("el"), HEBREW("iw"), HINDI(
			"hi"), HUNGARIAN("hu"), INDONESIAN("id"), ITALIAN("it"), JAPANESE(
			"ja"), KOREAN("ko"), LATVIAN("lv"), LITHUANIAN("lt"), MALTESE("mt"), NORWEGIAN(
			"no"), POLISH("pl"), PORTUGESE("pt"), ROMANIAN("ro"), RUSSIAN("ru"), SERBIAN(
			"sr"), SLOVAK("sk"), SLOVENIAN("sl"), SPANISH("es"), SWEDISH("sv"), THAI(
			"th"), TURKISH("tr"), UKRANIAN("uk"), VIETNAMESE("vi");
	private final String langName;

	private Language(String langName) {
		this.langName = langName;
	}

	@Override
	public String toString() {
		return this.langName;
	}

}

Kod jest już ładniejszy, prostszy i czytelniejszy. Zostało jeszcze posprzątać w kodzie stare metody i koniec.

Zyski, straty i brak zmian API

Jakie zyski przynosi tego typu refaktoryzacja. Przede wszystkim kod stał się prostszy nie ma już metody walidującej, bo walidacja została przerzucona na kompilator. Zamiast niesprawdzalnego Stringa użytkownik będzie mógł używać jedynie ściśle określonego zbioru języków. Każde odstępstwo będzie karane wywałką kompilatora.
Stratą może okazać się brak możliwości rozszerzenia klasy, ale oryginał też był finalny.
Ciekawostką jest tu fakt, że w klasach klienckich nie nastąpiła zmiana. Jeżeli ktoś używał zapisu Language.ENGLISH to nie musi zmieniać kodu ponieważ nazwa pola przeszła na nazwę enuma.
Kod innych należy jeszcze pozmieniać. Refaktoryzacja projektu trwa. Napisałem do właściciela kodu z prośbą o dopuszczenia do SVNa i zobaczymy co będzie.