How to write clean & testable code

Source code : https://github.com/ovy9086/clean-code

Transcript

1. How to write clean & testable code Ovidiu Latcu
2. Design a “LocationTracker” • gets new nearby places from the API • just if least “X” minutes passed since last check • if the user has moved at least “Y” meters • does not wake GPS if time constraint not met • returns an empty list if nothing found
3. Usual approach • looks like a good singleton candidate • single static `getInstance()` method • should be easy to implement • if we need other classes make them singleton as well • access other components via static getInstance()
4. public class LocationTracker { private static LocationTracker instance; private final Context context; private LocationTracker(Context context) { this.context = context; } public static LocationTracker getInstance(Context context) { if (instance == null) { instance = new LocationTracker(context) ; } return instance; } //... DbHelper.getInstance( context); //... } Keep reference to context Statically access all dependencies (which are static as well)
5. public class LocationTracker { private static LocationTracker instance; private DbHelper dbHelper; private LocationManager locationManager; private LocationTracker(Context context) { this.dbHelper = DbHelper.getInstance(context); this.locationManager = (LocationManager) context.getSystemService(Context. LOCATION_SERVICE); } public static LocationTracker getInstance(Context context) { if (instance == null) { instance = new LocationTracker(context) ; } return instance; } } Keep reference to dependencies Do work in the constructors to get them
6. public List<Place> checkNewNearbyPlaces() { //Implement... }
7. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System.currentTimeMillis() - DbHelper.getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //... }
8. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System. currentTimeMillis() - DbHelper. getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check LocationManager locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); Location lastLocation = locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER); //... }
9. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System. currentTimeMillis() - DbHelper. getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check LocationManager locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); Location lastLocation = locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER); Location lastUsedLocation = DbHelper.getInstance(context).getLastUsedLocation() if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //... }
10. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System.currentTimeMillis() - DbHelper. getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check LocationManager locationManager = (LocationManager) context.getSystemService(Context. LOCATION_SERVICE); Location lastLocation = locationManager.getLastKnownLocation(LocationManager. GPS_PROVIDER); Location lastUsedLocation = DbHelper. getInstance(context).getLastUsedLocation() if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check DbHelper.getInstance(context).setLastUsedLocation(lastLocation); DbHelper.getInstance(context).setLastCheck(System.currentTimeMillis()); return fetchPlacesFromApi(lastLocation); }
11. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System.currentTimeMillis() - DbHelper. getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check LocationManager locationManager = (LocationManager) context.getSystemService(Context. LOCATION_SERVICE); Location lastLocation = locationManager.getLastKnownLocation(LocationManager. GPS_PROVIDER); Location lastUsedLocation = DbHelper. getInstance(context).getLastUsedLocation(); if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check DbHelper.getInstance(context).setLastUsedLocation(lastLocation) ; DbHelper.getInstance(context).setLastCheck(System. currentTimeMillis()); return fetchPlacesFromApi(lastLocation) ; }
12. private List<Place> fetchPlacesFromApi(Location location) { OkHttpClient okHttpClient = new OkHttpClient() ; HttpUrl url = new HttpUrl.Builder().host( "http://mydomain.com ") .encodedPath( "/api/places") .addQueryParameter( "lat", "" + location.getLatitude()) .addQueryParameter( "lng", "" + location.getLongitude()) .build() ; try { Response response = okHttpClient.newCall( new Request.Builder().url(url).build()) .execute() ; Type listType = new TypeToken<ArrayList<Place>>() { }.getType() ; return new Gson().fromJson(response.body().string() , listType); } catch (IOException e) { e.printStackTrace() ; return new ArrayList<>(); } } Retreive the entries from the API
13. public class DbHelper extends SQLiteOpenHelper { private static final String CREATE_TABLE = "create table user_data (last_check int);"; private static DbHelper instance; public static DbHelper getInstance(Context context) { if (instance == null) { instance = new DbHelper(context); } return instance; } @Override public void onCreate(SQLiteDatabase db) { db.execSQL(CREATE_TABLE); } public long getLastCheck() { Cursor cursor = getReadableDatabase(). query("user_data", null, null, null, null, null, null); long result = cursor.getLong(0); cursor.close(); return result; } } Again a simple singleton for our DatabaseHelper
14. That looks good. Let’s try to test it
15. Hmmm… wait a minute now...do I need to • walk around the block to simulate 500 m distance change?
16. Hmmm… wait a minute now...do I need to • walk around the block to simulate 500 m distance change? • wait 1 hour until a new check is performed by the app ?
17. Hmmm… wait a minute now...do I need to • walk around the block to simulate 500 m distance change? • wait 1 hour until a new check is performed by the app ? • … I have a better idea ! I’ll unit test it !
18. @RunWith(MockitoJUnitRunner. class) public class BadLocationTrackerTest { @Test public void simpleTest() { LocationTracker tracker = LocationTracker. getInstance(mock(Context.class)); //Hmmm...not sure what to test... assertTrue(tracker.checkNewNearbyPlaces().size() > 0); } }
19. Ooops! java.lang.RuntimeException: Method getReadableDatabase in android.database.sqlite.SQLiteOpenHelper not mocked. See http://g.co/androidstudio/not-mocked for details. at android.database.sqlite.SQLiteOpenHelper.getReadableDatabase(SQLiteOpenHelper.java) at ro.corebuild.cleancode.bad.DbHelper.getLastCheck(DbHelper.java:48) at ro.corebuild.cleancode.bad.LocationTracker.checkNewNearbyPlaces(LocationTracker.java:49) at ro.corebuild.cleancode.BadLocationTrackerTest.simpleTest(BadLocationTrackerTest.java:24) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606)
20. The first line of code caused the crash public List<Place> checkNewNearbyPlaces() throws SecurityException { //Check if a enough time passed since last check if (System.currentTimeMillis() - DbHelper.getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { //... } }
21. The first line of code caused the crash public List<Place> checkNewNearbyPlaces() throws SecurityException { //Check if a enough time passed since last check if (System.currentTimeMillis() - DbHelper.getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { //... } } DbHelper.java public static DbHelper getInstance(Context context) { if (instance == null) { instance = new DbHelper(context); } return instance; }
22. The first line of code caused the crash public List<Place> checkNewNearbyPlaces() throws SecurityException { //Check if a enough time passed since last check if (System.currentTimeMillis() - DbHelper.getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { //... } } DbHelper.java public static DbHelper getInstance(Context context) { if (instance == null) { instance = new DbHelper(context); } return instance; } @Override public void onCreate(SQLiteDatabase db) { db.execSQL(CREATE_TABLE); }
23. What happened ? • the UnitTest is executed on the JVM (not a real device) • there is no context on the JVM • there is no real database on the JVM • everytime we access System components tests might crash
24. What’s wrong with the code ? • code violates the “Dependency Inversion Principle” ◦ “Depend upon Abstractions. Do not depend upon concretions.” • class has concrete dependencies - DbHelper & LocationManager • DbHelper & LocationManager are hidden dependencies • db storage & location dependencies can’t be stubbed / faked / mocked • same stands for the API dependency
25. Let’s extract our dependencies
26. Extract dependencies as interfaces public interface LocationProvider { Location getMostRecentLocation(); }
27. Extract dependencies as interfaces public interface LocationProvider { Location getMostRecentLocation(); } public class AndroidLocationProvider implements LocationProvider { private final LocationManager locationManager; public AndroidLocationProvider(LocationManager locationManager) { this.locationManager = locationManager; } @Override public Location getMostRecentLocation() { return locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER); } }
28. Extract dependencies as interfaces public interface UserStorage { void setLastCheck(long timeInMs); long getLastCheck(); void setLastUsedLocation(Location location); Location getLastUsedLocation(); } public class SqlUserStorage extends SQLiteOpenHelper implements UserStorage { //Implementation... }
29. Declare dependencies in the constructor public LocationTracker(LocationProvider locationProvider, UserStorage storage) { this.locationProvider = locationProvider; this.storage = storage; }
30. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System.currentTimeMillis() - DbHelper.getInstance(context).getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check LocationManager locationManager = (LocationManager) context.getSystemService(Context. LOCATION_SERVICE); Location lastLocation = locationManager.getLastKnownLocation(LocationManager. GPS_PROVIDER); Location lastUsedLocation = DbHelper.getInstance(context).getLastUsedLocation(); if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check DbHelper.getInstance(context).setLastUsedLocation(lastLocation) ; DbHelper.getInstance(context).setLastCheck(System. currentTimeMillis()); return fetchPlacesFromApi(lastLocation); }
31. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System.currentTimeMillis() - storage.getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check LocationManager locationManager = (LocationManager) context.getSystemService(Context. LOCATION_SERVICE); Location lastLocation = locationManager.getLastKnownLocation(LocationManager. GPS_PROVIDER); Location lastUsedLocation = storage.getLastUsedLocation(); if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check storage.setLastUsedLocation(lastLocation); storage.setLastCheck(System.currentTimeMillis()); return fetchPlacesFromApi(lastLocation); }
32. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System. currentTimeMillis() - storage.getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check LocationManager locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); Location lastLocation = locationManager.getLastKnownLocation(LocationManager.GPS_PROVIDER); Location lastUsedLocation = storage.getLastUsedLocation(); if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check storage.setLastUsedLocation(lastLocation); storage.setLastCheck(System. currentTimeMillis()); return fetchPlacesFromApi(lastLocation); }
33. public List<Place> checkNewNearbyPlaces() { //Check if a enough time passed since last check if (System. currentTimeMillis() - storage.getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check Location lastLocation = locationProvider.getMostRecentLocation(); Location lastUsedLocation = storage.getLastUsedLocation(); if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check storage.setLastUsedLocation(lastLocation); storage.setLastCheck(System. currentTimeMillis()); return fetchPlacesFromApi(lastLocation); }
34. public List<Place> checkNewNearbyPlaces () throws SecurityException { //Check if a enough time passed since last check if (System.currentTimeMillis() - storage.getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check Location lastLocation = locationProvider .getMostRecentLocation() ; Location lastUsedLocation = storage.getLastUsedLocation() ; if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check storage.setLastUsedLocation(lastLocation) ; storage.setLastCheck(System. currentTimeMillis()); return fetchPlacesFromApi(lastLocation) ; }
35. Looks better. Let’s try to test it
36. @Mock LocationProvider mockLocationProvider; @Mock UserStorage mockStorage; LocationTracker testedLocationTracker; @Before public void setup() { testedLocationTracker = new LocationTracker(mockLocationProvider, mockStorage); }
37. @Mock LocationProvider mockLocationProvider; @Mock UserStorage mockStorage; LocationTracker testedLocationTracker; @Before public void setup() { testedLocationTracker = new LocationTracker(mockLocationProvider, mockStorage); } @Test public void test_gpsNotQueried_ifMinTimeNotPassed() { //Given when(mockStorage.getLastCheck()).thenReturn(System.currentTimeMillis() - 2000); //When testedLocationTracker.checkNewNearbyPlaces(); //Then verifyZeroInteractions(mockLocationProvider); }
38. @Test public void test_gpsQueried_ifEnoughTimePassed() { //Given when(mockStorage.getLastCheck()).thenReturn(System.currentTimeMillis() - (LocationTracker.MIN_INTERVAL_MS + 5000)); //When testedLocationTracker.checkNewNearbyPlaces(); //Then verify(mockLocationProvider, times(1)).getMostRecentLocation(); }
39. Anything else to improve ? public List<Place> checkNewNearbyPlaces() throws SecurityException { //Check if a enough time passed since last check if (System.currentTimeMillis() - storage.getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check Location lastLocation = locationProvider.getMostRecentLocation(); Location lastUsedLocation = storage.getLastUsedLocation(); if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check storage.setLastUsedLocation(lastLocation); storage.setLastCheck(System.currentTimeMillis()); return fetchPlacesFromApi(lastLocation); }
40. Well… yes public List<Place> checkNewNearbyPlaces() throws SecurityException { //Check if a enough time passed since last check if (System.currentTimeMillis() - storage.getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check Location lastLocation = locationProvider.getMostRecentLocation(); Location lastUsedLocation = storage.getLastUsedLocation(); if (lastLocation.distanceTo(lastUsedLocation) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check storage.setLastUsedLocation(lastLocation); storage.setLastCheck(System.currentTimeMillis()); return fetchPlacesFromApi(lastLocation); } System dependency !
41. And also private List<Place> fetchPlacesFromApi(Location location) { OkHttpClient okHttpClient = new OkHttpClient(); HttpUrl url = new HttpUrl.Builder().host("http://mydomain.com").encodedPath("/api") .addQueryParameter("lat", "" + location.getLatitude()) .addQueryParameter("lng", "" + location.getLongitude()) .build(); try { Response response = okHttpClient.newCall( new Request.Builder().url(url).build()).execute(); Type listType = new TypeToken<ArrayList<Place>>() { }.getType(); return new Gson().fromJson(response.body().string(), listType); } catch (IOException e) { e.printStackTrace(); return new ArrayList<>(); } } API Dependency
42. More interfaces, yay ! public interface Clock { long timeInMillis(); }
43. More interfaces, yay ! public interface Clock { long timeInMillis(); } public class SystemClock implements Clock { @Override public long timeInMillis() { return System.currentTimeMillis(); } }
44. More interfaces, yay ! public interface Clock { long timeInMillis(); } public class SystemClock implements Clock { @Override public long timeInMillis() { return System.currentTimeMillis(); } } public interface PlacesApi { @GET("/places") List<Place> getPlaces(@Query("lat") double latitude, @Query("lng") double longitude); } Retrofit for our API
45. Refactor our class public DefaultLocationTracker(UserStorage storage, LocationProvider locationProvider, PlacesApi placesApi, Clock clock) { this.storage = storage; this.locationProvider = locationProvider; this.placesApi = placesApi; this.clock = clock; } Constructor declaring all the dependencies
46. Refactor our class public DefaultLocationTracker(UserStorage storage, LocationProvider locationProvider, PlacesApi placesApi, Clock clock) {...} @Override public List<Place> checkNearbyPlaces() { //Check if a enough time passed since last check if (clock.timeInMillis() - storage.getLastCheck() < MIN_INTERVAL_MS) { return new ArrayList<>(); } //Check if location has changed enough since last check Location location = locationProvider.getMostRecentLocation(); if (location.distanceTo(storage.getLastUsedLocation()) < MIN_DISTANCE_METERS) { return new ArrayList<>(); } //Store last location & check storage.setLastUsedLocation(location); storage.setLastCheck(System.currentTimeMillis()); return placesApi.getPlaces(location.getLatitude(), location.getLongitude()); }
47. Dependencies can be mocked / stubbed / faked now • complex scenarios can be simulated • easy to write unit tests • we can simulate location changes, movement • we can simulate time • we don’t need to wait for a dependency to be ready (eg: API) to continue / start development (we can mock it) (of course, you will be using some DI framework… Dagger)
48. public class FakeClock implements Clock { private long time; @Override public long timeInMillis() { return time; } public void setTime(long time) { this.time = time; } } public class FakeLocationProvider implements LocationProvider { private Location location; @Override public Location getMostRecentLocation() { return location; } public void setLocation(Location location) { this.location = location; } } Real handy fake implementations
49. public class FakePlacesApi implements PlacesApi { @Override public List<Place> getPlaces(@Query("lat") double latitude, @Query("lng") double longitude) { List<Place> places = new ArrayList<>(); for (int i = 0; i < 5; i++) { places.add(new Place()); } return places; } } public class MemoryUserStorage implements UserStorage { private long lastCheck; private Location lastLocation; @Override public void setLastUsedLocation(Location location) { this.lastLocation = location; } @Override public Location getLastUsedLocation() { return this.lastLocation; } //... } Real handy fake implementations
50. What did we gain besides unit tests • fake / stub / dummy implementations can be used in integration tests as well not only UNIT tests • implementations can be swapped at runtime • we can easily reproduce complex bugs
51. Take aways • depend on interfaces not implementations • avoid static singletons • extract all your dependencies • declare your dependencies
52. What dependencies to extract ? • storage, databases, file systems • time • GPS, location, sensors ... • bluetooth, NFC • external systems • APIs
53. Thanks. Questions ?