Давайте посмотрим на этот кусок кода:
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
|
public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException { List<Trip> tripList = new ArrayList<Trip>(); User loggedUser = UserSession.getInstance().getLoggedUser(); boolean isFriend = false; if (loggedUser != null) { for (User friend : user.getFriends()) { if (friend.equals(loggedUser)) { isFriend = true; break; } } if (isFriend) { tripList = TripDAO.findTripsByUser(user); } return tripList; } else { throw new UserNotLoggedInException(); }} |
Ужасно, не так ли? Приведенный выше код имеет множество проблем, но прежде чем мы его изменим, мы должны покрыть его тестами.
При модульном тестировании по методу выше есть две проблемы. Они есть:
|
1
2
3
|
User loggedUser = UserSession.getInstance().getLoggedUser(); // Line 3 tripList = TripDAO.findTripsByUser(user); // Line 13 |
Как мы знаем, модульные тесты должны тестировать только один класс, а не его зависимости. Это означает, что нам нужно найти способ издеваться над Синглтоном и статическим вызовом. В общем, мы делаем это, вводя зависимости, но у нас есть правило , помните?
Мы не можем изменить существующий код, если он не покрыт тестами. Единственное исключение — если нам нужно изменить код, чтобы добавить модульные тесты, но в этом случае допускаются только автоматические рефакторинги (через IDE).
Кроме того, многие из фальшивых фреймворков в любом случае не могут имитировать статические методы, поэтому внедрение TripDAO не решит проблему.
Преодоление проблемы жестких зависимостей
ПРИМЕЧАНИЕ. В реальной жизни я бы сначала писал тесты и вносил изменения именно тогда, когда мне было нужно, но для того, чтобы пост был коротким и целенаправленным, я не буду идти здесь шаг за шагом.
Прежде всего, давайте выделим зависимость Singleton от его собственного метода. Давайте сделаем это защищенным. Но подождите, это нужно сделать с помощью автоматического рефакторинга «метод извлечения». Выберите только следующий фрагмент кода на TripService.java:
|
1
|
UserSession.getInstance().getLoggedUser() |
Перейдите в меню рефакторинга вашей IDE, выберите метод извлечения и присвойте ему имя. После этого шага код будет выглядеть так:
|
01
02
03
04
05
06
07
08
09
10
11
12
|
public class TripService { public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException { ... User loggedUser = loggedUser(); ... } protected User loggedUser() { return UserSession.getInstance().getLoggedUser(); }} |
Делая то же самое для TripDAO.findTripsByUser (пользователь), мы получим:
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
|
public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException { ... User loggedUser = loggedUser(); ... if (isFriend) { tripList = findTripsByUser(user); } ...} protected List<Trip> findTripsByUser(User user) { return TripDAO.findTripsByUser(user);} protected User loggedUser() { return UserSession.getInstance().getLoggedUser();} |
В нашем тестовом классе мы можем теперь расширить класс TripService и переопределить созданные нами защищенные методы, заставляя их возвращать все, что нам нужно для наших модульных тестов:
|
01
02
03
04
05
06
07
08
09
10
|
private TripService createTripService() { return new TripService() { @Override protected User loggedUser() { return loggedUser; } @Override protected List<Trip> findTripsByUser(User user) { return user.trips(); } };} |
И это все. Наш TripService теперь тестируемый.
Сначала мы пишем все тесты, которые нам нужны, чтобы убедиться, что класс / метод полностью протестирован и все ветви кода выполнены. Для этого я использую плагин Eclipse eclEmma , и я настоятельно рекомендую его. Если вы не используете Java и / или Eclipse, попробуйте использовать инструмент покрытия кода, специфичный для вашего языка / IDE, при написании тестов для существующего кода. Это очень помогает.
Итак, вот мой последний тестовый класс:
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
|
public class TripServiceTest { private static final User UNUSED_USER = null; private static final User NON_LOGGED_USER = null; private User loggedUser = new User(); private User targetUser = new User(); private TripService tripService; @Before public void initialise() { tripService = createTripService(); } @Test(expected=UserNotLoggedInException.class) public void shouldThrowExceptionWhenUserIsNotLoggedIn() throws Exception { loggedUser = NON_LOGGED_USER; tripService.getTripsByUser(UNUSED_USER); } @Test public void shouldNotReturnTripsWhenLoggedUserIsNotAFriend() throws Exception { List<Trip> trips = tripService.getTripsByUser(targetUser); assertThat(trips.size(), is(equalTo(0))); } @Test public void shouldReturnTripsWhenLoggedUserIsAFriend() throws Exception { User john = anUser().friendsWith(loggedUser) .withTrips(new Trip(), new Trip()) .build(); List<Trip> trips = tripService.getTripsByUser(john); assertThat(trips, is(equalTo(john.trips()))); } private TripService createTripService() { return new TripService() { @Override protected User loggedUser() { return loggedUser; } @Override protected List<Trip> findTripsByUser(User user) { return user.trips(); } }; } } |
Мы все?
Конечно нет. Нам все еще нужно провести рефакторинг класса TripService.
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
|
public class TripService { public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException { List<Trip> tripList = new ArrayList<Trip>(); User loggedUser = loggedUser(); boolean isFriend = false; if (loggedUser != null) { for (User friend : user.getFriends()) { if (friend.equals(loggedUser)) { isFriend = true; break; } } if (isFriend) { tripList = findTripsByUser(user); } return tripList; } else { throw new UserNotLoggedInException(); } } protected List<Trip> findTripsByUser(User user) { return TripDAO.findTripsByUser(user); } protected User loggedUser() { return UserSession.getInstance().getLoggedUser(); }} |
Сколько проблем вы видите? Не торопитесь, прежде чем читать те, которые я нашел .. 🙂
Рефакторинг
ПРИМЕЧАНИЕ . Когда я это сделал, я делал это шаг за шагом, выполняя тесты после каждого шага. Здесь я просто суммирую мои решения .
Первое, что я заметил, — то, что переменная tripList не должна быть создана, когда зарегистрированный пользователь является нулем, так как исключение выдается, и больше ничего не происходит. Я решил инвертировать внешний if и извлечь пункт охраны .
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
|
public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException { User loggedUser = loggedUser(); validate(loggedUser); List<Trip> tripList = new ArrayList<Trip>(); boolean isFriend = false; for (User friend : user.getFriends()) { if (friend.equals(loggedUser)) { isFriend = true; break; } } if (isFriend) { tripList = findTripsByUser(user); } return tripList;}private void validate(User loggedUser) throws UserNotLoggedInException { if (loggedUser == null) throw new UserNotLoggedInException();} |
Зависть
Когда класс получает данные из другого класса для выполнения каких-либо вычислений или сравнения этих данных, довольно часто это означает, что клиентский класс завидует другому классу. Это называется Feature Envy (запах кода), и это часто встречается в длинных методах и повсеместно встречается в устаревшем коде. В ОО данные и операции над этими данными должны находиться на одном объекте.
Итак, глядя на приведенный выше код, ясно, что все, что касается определения, дружит ли пользователь с другим, не относится к классу TripService. Давайте переместим его в класс User. Сначала юнит тест:
|
1
2
3
4
5
6
7
8
9
|
@Test public voidshouldReturnTrueWhenUsersAreFriends() throws Exception { User John = new User(); User Bob = new User(); John.addFriend(Bob); assertTrue(John.isFriendsWith(Bob));} |
Теперь давайте переместим код в класс User. Здесь мы можем немного лучше использовать API коллекций Java и полностью удалить цикл for и флаг isFriend.
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
|
public class User { ... private List<User> friends = new ArrayList<User>(); public void addFriend(User user) { friends.add(user); } public boolean isFriendsWith(User friend) { return friends.contains(friend); } ...} |
После нескольких шагов рефакторинга, вот новый код в TripService
|
1
2
3
4
5
6
7
|
public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException { User loggedUser = loggedUser(); validate(loggedUser); return (user.isFriendsWith(loggedUser)) ? findTripsByUser(user) : new ArrayList<Trip>();} |
Правильно. Это уже намного лучше, но все еще недостаточно хорошо.
Слои и зависимости
Некоторых из вас все еще могут раздражать защищенные методы, которые мы создали в первой части , чтобы изолировать зависимости и протестировать класс. Подобные изменения должны быть временными, это означает, что они сделаны, чтобы мы могли модульно протестировать весь метод. Как только у нас появятся тесты, охватывающие метод, мы можем начать рефакторинг и подумать о зависимостях, которые мы можем внедрить.
Много раз мы думали, что мы должны просто ввести зависимость в класс. Это звучит очевидно. TripService должен получить экземпляр UserSession. В самом деле?
TripService — это сервис. Это означает, что он находится на уровне сервиса. UserSession знает о зарегистрированных пользователях и сеансах. Вероятно, он взаимодействует с платформой MVC и / или HttpSession и т. Д. Должен ли TripService зависеть от этого класса (даже если это был интерфейс, а не Singleton)? Вероятно, вся проверка, вошел ли пользователь в систему, должна выполняться контроллером или каким-либо другим классом клиента. Для того, чтобы НЕ изменять это так много (пока), я заставлю TripService принять зарегистрированного пользователя в качестве параметра и полностью удалим зависимость от UserSession. Мне нужно будет внести небольшие изменения и очистить тесты.
Именование
Нет, к сожалению, мы еще не закончили. Что этот код делает в любом случае? Обратные поездки от друга. Глядя на имя метода и параметров, или даже имя класса, нет никакого способа узнать это. Слово «друг» — это не то, что можно увидеть в общедоступном интерфейсе TripService. Мы должны изменить это также.
Итак, вот окончательный код:
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
|
public class TripService { public List<Trip> getFriendTrips(User loggedUser, User friend) throws UserNotLoggedInException { validate(loggedUser); return (friend.isFriendsWith(loggedUser)) ? findTripsForFriend(friend) : new ArrayList<Trip>(); } private void validate(User loggedUser) throws UserNotLoggedInException { if (loggedUser == null) throw new UserNotLoggedInException(); } protected List<Trip> findTripsForFriend(User friend) { return TripDAO.findTripsByUser(friend); }} |
Лучше не так ли? У нас все еще есть проблема с другим защищенным методом, со статическим вызовом TripDAO и т. Д. Но я оставлю этот последний бит для другого поста о том, как удалить зависимости от статических методов. Я пока оставлю свой рефакторинг. Мы не можем провести рефакторинг всей системы за один день, верно? Нам все еще нужно доставить некоторые функции. 🙂
Вывод
Это был просто игрушечный пример и даже не имеет смысла. Тем не менее, он представляет многие проблемы, которые мы обнаруживаем при работе с устаревшим (существующим) кодом. Удивительно, сколько проблем мы можем найти в таком крошечном кусочке кода. Теперь представьте все эти классы и методы с сотнями, если не тысячами строк.
Нам нужно продолжать беспощадно рефакторинг нашего кода, чтобы мы никогда не попадали в положение, когда мы его больше не понимаем, и весь бизнес начинает замедляться, потому что мы не можем достаточно быстро настроить программное обеспечение.
Рефакторинг — это не просто извлечение методов или внесение нескольких изменений в логику. Нам нужно подумать о зависимостях, обязанностях, которые должен иметь каждый класс и метод, об архитектурных уровнях, дизайне нашего приложения, а также об именах, которые мы даем каждому классу, методу, параметру и переменной. Мы должны попытаться сделать бизнес-домен выраженным в коде.
Мы должны относиться к нашей базе кода как к большому саду . Если мы хотим, чтобы это было приятным и ремонтопригодным, мы должны постоянно заботиться о нем.
Если вы хотите попробовать этот код или найти более подробную информацию о реализации, проверьте: https://github.com/sandromancuso/testing_legacy_code
Ссылка: Тестирование прежних версий: жесткие зависимости (часть 1 и 2) от нашего партнера JCG Сандро Манкузо из блога Crafted Software .