O elegancji kodu

W tym tygodniu będziemy pracować z kodem ze wczorajszego posta.

O co chodzi?

Wczorajszy post powstał na szybko, bo Mati miał urodziny i nie miałem czasu się rozpisywać. Jednak już na pierwszy rzut oka wiadomo, że kod tak napisany, choć spełnia założenia tzn. wypisuje szansę wylosowania danej kuli w totka, nie jest dobry. Jest stosunkowo prosty. Łączy w sobie elementy imperatywne i deklaratywne, a całość doprawiła Java 8.

W tym tygodniu celem jest takie jego przepisanie by uzyskać kod możliwie jak najbardziej elastyczny, testowalny i rozszerzalny. Przy jednoczesnym zachowaniu jego prostoty. Oczywiście całość zgodnie z zasadami TDD itp. itd.

Początek

Nasz program ma za zadanie symulować prostą grę liczbową. Z ograniczonego zbioru liczb/kul/symboli itp. losujemy n elementów. W zależności od reguł gry elementy mogą się powtarzać albo nie. Dlatego też nasz kod musi być jak najbardziej elastyczny.

Maszyna losująca

Na samym początku stwórzmy klasę reprezentującą generyczną maszynę losującą. Jej zadaniem jest zamknięcie w sobie opisanego wyżej schematu gry. Na potrzeby testów niech nasza gra, będzie wylosowaniem jednego elementu ze jednoelementowego zbioru liczb.

Pierwszy test będzie zatem wyglądał tak:

Listing 1. Pierwszy test

public class MachineTest {

	Machine<Integer> sut == new Machine<>() {
		@Override
		public Integer pick() {
			return null;
		}
	};

	@Test
	public void shouldPickOneOfOne() throws Exception {

		Integer result = sut.pick();

		Assertions.assertThat(result).isEqualTo(1);
	}
}

Oczywiście całość radośnie się wywali. Metoda pick jest abstrakcyjna, dlatego znalazła się w teście. Zamieńmy ją na konkretną, dzięki czemu powstanie kod maszyny, który będziemy mogli rozwijać:

Listing 2. Maszyna I

public abstract class Machine<T>  {

	public T pick() {
		return null;
	}
}

Wróćmy na chwilę do naszego zdefiniowanego zachowania. Maszyna ze zbioru wybiera n elementów. Musi zatem istnieć jakaś metoda, która przyjmie nam zbiór wejściowy oraz liczbę elementów do wylosowania. Metoda pick zdaje się być niezła do tego. Przy najmniej na razie 😉 Zmieńmy zatem test oraz klasę Machine.

Listing 3. Zmieniony test

public class MachineTest {

	Machine<Integer> sut = new Machine<>(){};

	@Test
	public void shouldPickOneOfOne() throws Exception {

		Collection<Integer> input = Lists.newArrayList(1);

		Integer result = sut.pick(input, 1);

		Assertions.assertThat(result).isEqualTo(1);
	}
}

Test nadal nie działa, ale już wiemy przynajmniej z jakimi danymi przyjdzie nam się zmierzyć. Możemy zatem zaimplementować coś więcej niż tylko zwracanie null.

Listing 2. Maszyna II

public abstract class Machine<T>{

	public T pick(Collection<T> input, int i) {
		return
				map(input.stream().limit(i).collect(Collectors.toList()));
	}

	protected abstract T map(List<T> drawn);
}

Nasz test wymaga jeszcze jednej drobnej korekty by zaświecić się na zielono. Musimy zaimplementować metodę map.

Listing 5. Zielony test

public class MachineTest {

	Machine<Integer> sut = new Machine<Integer>(){
		@Override
		protected Integer map(List<Integer> drawn) {
			return drawn.get(0);
		}
	};

	@Test
	public void shouldPickOneOfOne() throws Exception {

		Collection<Integer> input = Lists.newArrayList(1);

		Integer result = sut.pick(input, 1);

		Assertions.assertThat(result).isEqualTo(1);
	}
}

Na tym etapie widać już, że nasza koncepcja z przekazywaniem parametrów jest zła. Metoda map otrzymuje wylosowany podzbiór i powinna zamienić go na obiekt reprezentujący wynik. U nas wynikiem jest pojedyncza liczba. Poza tym nie mamy jeszcze elementu losowego! Zatem potrzebujemy dwóch rzeczy:

• Funkcjonalności, która pomiesza dane wejściowe w losowy sposób.
• Funkcjonalności, która zamieni podzbiór wylosowanych elementów w wynik.

Drugi element w formie paskudnej, ale już mamy w postaci metody map.

Pierwszy element jest własnością wszystkich maszyn, bo pamiętajmy, że choć piszemy abstrakcyjną maszynę to ma ona pewną funkcjonalność. Najłatwiej będzie opisać taką funkcjonalność za pomocą funkcji, która jako parametr wejściowy przyjmuje jakiś zbiór i zwraca zbiór po „wstrząśnięciu”. Tu mała uwaga, jak zwykle komputery nie są doskonałe i zbiory, które są reprezentowane przez kolekcje różnego rodzaju zazwyczaj mają uporządkowaną (w jakiś sposób) strukturę. Może być to posortowanie, kolejność wstawiania czy też uporządkowanie według jakiejś właściwości zatem nie mówimy to o zbiorach w ściśle matematycznym podejściu. Raczej o listach, kolekcjach, zbiorach (Set) itp.

Zaimplementujmy zatem tą funkcjonalność. Oczywiście najpierw test. Tylko jak przetestować losowość? No dobra… nie da się 😉 Przynajmniej w prosty sposób. Zatem nie frasujmy się tym problemem i oddelegujmy go do API języka. Naszym zadaniem będzie zatem tylko odpowiednie użycie API tak by nic nie zepsuć.

Przez odpowiednie użycie rozumiem tu takie wykorzystanie metody Collections.shuffle, by nie zmienić danych wejściowych. Zatem musimy sprawdzić czy maszyna gdzieś pod spodem dokonuje jakiejś formy kopiowania danych i w czasie losowania wykorzystuje kopię.

Listing 6. Kolejny test

public class MachineTest {

//...

	@Test
	public void shouldRandomizeCopyOfInput() throws Exception {
		List<Integer> input = Lists.newArrayList(1, 2, 3, 4, 5);

		Integer result = sut.pick(input, 1);

		Assertions.assertThat(result).isIn(1, 2, 3, 4, 5);
		Assertions.assertThat(input).hasSize(5).containsExactly(1, 2, 3, 4, 5);

	}
}

Pierwsza asercja sprawdza czy wylosowany element zawiera się w zbiorze. Jest to smoke test, który nic na chwilę obecną nie robi, ale jeżeli wywali się to oznacza iż maszyna poza zmianą kolejności wejścia też coś od siebie dodała. Druga asercja sprawdza czy dane wejściowe po wywołaniu metody zachowały kolejność. Dodajmy zatem odpowiedni kod do naszej maszyny:

Listing 7. Maszyna III

public abstract class Machine<T>{

	public T pick(Collection<T> input, int i) {

		List<T> list = new ArrayList<>(input);
		Collections.shuffle(list);

		return
				map(input.stream().limit(i).collect(Collectors.toList()));
	}

	protected abstract T map(List<T> drawn);
}

I git. Mamy element kopiowania defensywnego, mamy element losowości.

O elegancji kodu

Kod z listingu 7 w praktyce robi wszystko co potrzeba. Będzie on odpowiednikiem draw, z poprzedniego wpisu. Nakarmiony danymi zwróci co trzeba. Z drugiej strony ujawnia za wiele co do samej maszyny. Wyobraźmy sobie, że ktoś tworzy maszynę do klasycznego Dużego Lotka. Chciałby aby użytkownicy przestrzegali reguł, czyli na wejściu podali listę 49 kolejnych liczb oraz 6 jako ilość losowanych elementów. Następnie umieszczamy naszą maszynę w jakiejś kolekcji i przechodząc przez nią za każdym razem losujemy… no właśnie co? Nie za bardzo można użyć naszej obecnej implementacji jako abstrakcji. Zmiana danych przez użytkownika wpłynie na wyniki co może powodować, że rozszerzanie tej klasy nie ma sensu! Skoro użytkownik może podać dowolne dane wejściowe, to po co nam podklasy?
Można oczywiście nadać naszej metodzie pick widoczność protected i wprowadzić dodatkową metodę pick, która nie miała by żadnych parametrów i delegowała by odpowiednie zachowanie do tej pierwszej podając parametry. Fajnie, ale nadal nie jest to kod elegancki. W dodatku obecny kod metody pick robi wszystko. Niech zatem opisuje tylko schemat działania, a wszystko inne pozostawi klasom dziedziczącym. W dodatku mamy tu jakieś zagnieżdżone wywołania… a fuj…

Kod elegancki powinno dać czytać się jak zwyczajny tekst. No prawie, bo trzeba by dodać pewien element interpretacji, który dopasowuje gramatykę języka programowania do naszej. Z drugiej strony:

Weź kolekcję danych wejściowych, następnie stwórz jej kopię, następnie zmieszaj kopię, następnie wybierz n elementów, następnie zamień je na rezultat, zwróć rezultat.

Da się, choć ciężko się to czyta. Spróbujmy zaimplementować takiego językowego potworka 😉 Inaczej mówiąc zrefaktoryzujmy naszą metodę tak by jej kod pasował do powyższego opisu. Na tym etapie nie dotykamy testów. Za wyjątkiem zmiany API metody pick.

W naszym kodzie wykorzystam dwie funkcje z jednego z poprzednich wpisów – Function0 i VoidFunction.

Listing 8. Maszyna IV

public abstract class Machine<IN, T>{

	public T pick() {
		return getData()
				.andThen(defensiveCopy())
				.andThen(shuffle())
				.andThen(drawN())
				.andThen(mapToResult())
				.apply(null);
	}

	protected Function<Collection<IN>, List<IN>> defensiveCopy() {
		return ArrayList::new;
	}

	protected Function<List<IN>, List<IN>> shuffle() {
		return l -> {
			Collections.shuffle(l);
			return l;
		};
	}

	protected BiFunction<List<IN>, Integer, List<IN>> draw() {
		return (c, l) -> c.stream().limit(l).collect(Collectors.toList());
	}

	protected Function<List<IN>, List<IN>> drawN() {
		return l -> draw().apply(l, limit());
	}
}

Metody getData, defensiveCopy, shuffle, drawN i mapToResult, będą zwracać kolejne funkcje reprezentujące kroki w losowaniu, a limit dostarcza nam konkretnej wartości. Dzięki temu każdy element algorytmu może zostać wymieniony w poszczególnych implementacjach. Oczywiście część elementów jest wspólna na przykład defensiveCopy i shuffle (choć tu można pokusić się o własną implementację, która z definicji nie dotyka wejścia), czy też wybieranie n elementów.

Testy też uległy modyfikacjom.

Listing 9. Testy po zmianach

public class MachineTest {

	@Test
	public void shouldPickOneOfOne() throws Exception {

		Machine<Integer, Integer> sut = new Machine<Integer, Integer>() {

			@Override
			protected Function0<Collection<Integer>> getData() {
				return () -> Lists.newArrayList(1);
			}

			@Override
			protected Function<Collection<Integer>, Integer> mapToResult() {
				return c -> c.stream().findFirst().get();
			}

			@Override
			protected Integer limit() {
				return 1;
			}
		};


		Integer result = sut.pick();

		Assertions.assertThat(result).isEqualTo(1);
	}

	@Test
	public void shouldRandomizeCopyOfInput() throws Exception {
		ArrayList<Integer> input = Lists.newArrayList(1, 2, 3, 4, 5);

		Machine<Integer, Integer> sut = new Machine<Integer, Integer>() {

			@Override
			protected Function0<Collection<Integer>> getData() {
				return () -> input;
			}

			@Override
			protected Function<Collection<Integer>, Integer> mapToResult() {
				return c -> c.stream().findFirst().get();
			}

			@Override
			protected Integer limit() {
				return 1;
			}
		};


		Integer result = sut.pick();

		Assertions.assertThat(result).isIn(1, 2, 3, 4, 5);
		Assertions.assertThat(input). hasSize(5).containsExactly(1, 2, 3, 4, 5);

	}
}

Czy rozwiązanie z listingu 8 jest bardziej eleganckie niż rozwiązanie z listingu 7? Na pewno, choć wiem, że dla niektórych osób przeczytanie nowej wersji kodu będzie na początku trudniejsze. Z drugiej strony gdy popatrzymy na testy są one znacznie prostsze. Jawnie określają, które elementy maszyny chcemy przetestować. Co więcej odpowiednio komponując pewne funkcje możemy sprawdzić każdy z kroków oddzielnie bez konieczności uruchamiania całego procesu losowania.

Podsumowanie

W tej części skupiliśmy się na napisaniu maszyny losującej w taki sposób można było ją łatwo dostosowywać do konkretnych potrzeb. W praktyce możemy dzięki niej zbudować dowolną grę losową. W kolejnej części przejdziemy do klasy Lotto, w której wykorzystamy naszą maszynę do budowy statystyki totolotka. Następne wpisy będą poświęcone budowie aplikacji do gry „systemowej” 😉