clean code via dependency injection + guice
DESCRIPTION
From Google DevFest 2012, BarcelonaTRANSCRIPT
Clean code via Dependency Injection + Guice
@jordi99 noviembre 2012bit.ly/devfest12cleancode
... shameless self-promotion bro!
About you
Gang of Four?Legacy Code?Dependency Injection?Monkey patching?Monkey testing?Unit testing?TDD / BDD?Agile? XP? Scrum what?
S.O.L.I.D.
Single responsibility principleOpen/closed principleLiskov substitution principleInterface segregation principleDependency inversion principle
Legacy code is code without test
Fast Run hundreds or thousands per second
Isolated Failure reasons become obvious
Reliable Run repeatedly in any order, any time
Self-validating No manual evaluation required
Timely Written before the code
F.I.R.S.T.
Write some code, kill technical debt(or not)
class Showtimes {
MovieRepository repository;
Showtimes() {
repository = MovieRepositoryFactory.getInstance();
}
Set<Movie> findMovies(City city) { return repository.findByCity(city);
}
}
Showtime!
class Showtimes {
MovieRepository repository;
Showtimes() {
repository = MovieRepositoryFactory.getInstance();
}
Set<Movie> findMovies(City city) { return repository.findByCity(city);
}
}
Smell: Constructor does real work
class Showtimes {
MovieRepository repository;
Showtimes() {
repository = MovieRepositoryFactory.getInstance(); }
Set<Movie> findMovies(City city) { return repository.findByCity(city);
}
}
Smell: Constructor does real work
class Showtimes {
MovieRepository repository;
Showtimes() {
repository = MovieRepositoryFactory.getInstance(); }
Set<Movie> findMovies(City city) { return repository.findByCity(city);
}
}
Smell: Constructor does real work
Factory Pattern, boilerplate
public class MovieRepositoryFactory { static class MovieRepositoryHolder {
static MovieRepository instance = new MovieRepositoryImpl(); }
public static MovieRepository getInstance() { return MovieRepositoryHolder.instance;
}
static void setInstance(MovieRepository mock) { MovieRepositoryHolder.instance = mock;
}
}
Factory Pattern, boilerplate
public class MovieRepositoryFactory { static class MovieRepositoryHolder {
static MovieRepository instance = new MovieRepositoryImpl(); }
public static MovieRepository getInstance() { return MovieRepositoryHolder.instance;
}
static void setInstance(MovieRepository mock) { MovieRepositoryHolder.instance = mock;
}
}
Factory Pattern, boilerplate
public class MovieRepositoryFactory { static class MovieRepositoryHolder {
static MovieRepository instance = new MovieRepositoryImpl(); }
public static MovieRepository getInstance() { return MovieRepositoryHolder.instance;
}
static void setInstance(MovieRepository mock) { MovieRepositoryHolder.instance = mock;
}
}
Factory Pattern, boilerplate
public class MovieRepositoryFactory { static class MovieRepositoryHolder {
static MovieRepository instance = new MovieRepositoryImpl(); }
public static MovieRepository getInstance() { return MovieRepositoryHolder.instance;
}
static void setInstance(MovieRepository mock) { MovieRepositoryHolder.instance = mock;
}
}
Factory Pattern, boilerplate
public class MovieRepositoryFactory { static class MovieRepositoryHolder { static MovieRepository instance = new MovieRepositoryImpl(); }
public static MovieRepository getInstance() { return MovieRepositoryHolder.instance;
}
static void setInstance(MovieRepository mock) { MovieRepositoryHolder.instance = mock;
}
}
Factory Pattern, boilerplate
public class MovieRepositoryFactory { static class MovieRepositoryHolder { static MovieRepository instance = new MovieRepositoryImpl(); }
public static MovieRepository getInstance() { return MovieRepositoryHolder.instance;
}
static void setInstance(MovieRepository mock) { MovieRepositoryHolder.instance = mock;
}
}
@Test
void findMovies_cantTestYouDude() { Showtimes showtimes = new Showtimes();
// FRAK!
// No database setup... I'm stuck with my collaborators
// Can't really change it... Will it run in my
// partners computer?
}
Hard to test
Fix: Constructor does real work
class Showtimes {
MovieRepository repository;
Showtimes(MovieRepository repository) {
this.repository = repository;
}
Set<Movie> findMovies(City city) { return repository.findByCity(city);
}
}
Fix: Constructor does real work
class Showtimes {
// Hollywood principle: "Don't call us, we'll call you"
MovieRepository repository;
Showtimes(MovieRepository repository) {
this.repository = repository;
}
Set<Movie> findMovies(City city) { return repository.findByCity(city);
}
}
Fix: Constructor does real work
class Showtimes {
// Hollywood principle: "Don't call us, we'll call you"
MovieRepository repository;
@Inject Showtimes(MovieRepository repository) {
this.repository = repository;
}
Set<Movie> findMovies(City city) { return repository.findByCity(city);
}
}
Fix: Constructor does real work
Fix: MovieRepository creation?
class App {
public static void main(String[] args) { // Manual Dependency Injection... but much more
// boilerplate!
// Maybe some <init> method... but that's why
// constructors exist right?
// Some container injection... mmm... Spring, Pico?
// Or better, Guice time! }
}
class App {
public static void main(String[] args) { Injector injector = Guice.createInjector( new AbstractModule() {
@Override void configure() { bind(MovieRepository.class)
.to(JpaMovieRepository.class);
}
});
injector.getInstance(Showtimes.class).findMovies(bcn);
}
}
Fix: MovieRepository creation
class App {
public static void main(String[] args) { Injector injector = Guice.createInjector( new AbstractModule() {
@Override void configure() { bind(MovieRepository.class) .to(JpaMovieRepository.class);
}
});
injector.getInstance(Showtimes.class).findMovies(bcn);
}
}
Fix: MovieRepository creation
class App {
public static void main(String[] args) { Injector injector = Guice.createInjector( new AbstractModule() {
@Override void configure() { bind(MovieRepository.class) .to(JpaMovieRepository.class);
}
});
// Injection is VIRAL! Avoid at all cost using the // Injector in more than one place (aka Service Locator)
injector.getInstance(Showtimes.class).findMovies(bcn); }
}
Fix: MovieRepository creation
Showtimes showtimes;
@Mock MovieRepository repository;
@Test void findMovies_easyNow() { showtimes = new Showtimes(repository);
given(repository.findByCity(bcn)) .willReturn(ImmutableSet.of(looper));
Set<Movie> movies = showtimes.findMovies(bcn);
assertThat(movies, hasItem(looper));}
Fix: Testable and flexible design
Showtimes showtimes; //SUT
@Mock MovieRepository repository;
@Test void findMovies_easyNow() { showtimes = new Showtimes(repository);
given(repository.findByCity(bcn)) .willReturn(ImmutableSet.of(looper));
Set<Movie> movies = showtimes.findMovies(bcn);
assertThat(movies, hasItem(looper));}
Fix: Testable and flexible design
Showtimes showtimes; //SUT
@Mock MovieRepository repository; // <3 Mockito, BDD
@Test void findMovies_easyNow() { showtimes = new Showtimes(repository);
//given
given(repository.findByCity(bcn)) .willReturn(ImmutableSet.of(looper)); //when
Set<Movie> movies = showtimes.findMovies(bcn);
//then
assertThat(movies, hasItem(looper));}
Fix: Testable and flexible design
Showtimes showtimes; //SUT
@Mock MovieRepository repository; // <3 Mockito, BDD
@Test void findMovies_easyNow() { showtimes = new Showtimes(repository);
//given
given(repository.findByCity(bcn)) .willReturn(ImmutableSet.of(looper)); //when
Set<Movie> movies = showtimes.findMovies(bcn);
//then
assertThat(movies, hasItem(looper)); // <3 Hamcrest}
Fix: Testable and flexible design
Showtimes showtimes; //SUT
@Mock MovieRepository repository; // <3 Mockito, BDD
@Test void findMovies_easyNow() { showtimes = new Showtimes(repository);
//given
given(repository.findByCity(bcn)) .willReturn(ImmutableSet.of(looper)); // <3 Guava //when
Set<Movie> movies = showtimes.findMovies(bcn);
//then
assertThat(movies, hasItem(looper)); // <3 Hamcrest}
Fix: Testable and flexible design
Constructor does real work, more flavours
class Ratings {
User user;
Ratings(AccountManager manager) {
user = manager.getLoggedUser();
user.setQueue(new ItTakesForeverToBuildOSSQueue());
}
}
Flavour: Partial objects, Law of Demeter
class Ratings {
User user;
Ratings(AccountManager manager) {
user = manager.getLoggedUser(); user.setQueue(new ItTakesForeverToBuildOSSQueue()); }
// Test? Slow, really slow
// We don't use AccountManager anywhere else
}
Flavour: Partial objects, Law of Demeter
class Ratings {
User user;
@Inject Ratings(User user) {
this.user = user;
}
}
Fix: Partial objects, Law of Demeter
class Ratings {
User user;
// How do we inject this User like we need it?
// Where's the Queue?
@Inject Ratings(User user) {
this.user = user;
}
}
Fix: Partial objects, Law of Demeter
@Provides
User providesUser(AccountManager manager, Queue queue) {
User user = manager.getLoggedUser();
user.setQueue(queue);
return user;
}
@Provides @Singleton
Queue providesLongRunningQueue() {
return new ItTakesForeverToBuildOSSQueue();
}
Fix: Partial objects, Law of Demeter
@ProvidesUser providesUser(AccountManager manager, Queue queue) {
User user = manager.getLoggedUser();
user.setQueue(queue);
return user;
}
@Provides @Singleton
Queue providesLongRunningQueue() {
return new ItTakesForeverToBuildOSSQueue();
}
Fix: Partial objects, Law of Demeter
@ProvidesUser providesUser(AccountManager manager, Queue queue) {
User user = manager.getLoggedUser();
user.setQueue(queue);
return user;
}
@Provides @Singleton // no JVM Singleton, yay!Queue providesLongRunningQueue() {
return new ItTakesForeverToBuildOSSQueue();
}
Fix: Partial objects, Law of Demeter
@ProvidesUser providesUser(AccountManager manager, Queue queue) {
User user = manager.getLoggedUser();
user.setQueue(queue);
return user;
}
@Provides @Singleton // no JVM Singleton, yay!Queue providesLongRunningQueue() {
return new ItTakesForeverToBuildOSSQueue();
}
// Provider methods should be inside a Module
Fix: Partial objects, Law of Demeter
Ratings ratings; //SUT
@Test void ratings_findAverage() { ratings = new Ratings(new DummyUser());
// Easy to test now, we can have a mock or test-double
// and check everything about Ratings. We don't care
// about collaborators.
}
Test: Partial objects, Law of Demeter
Warning signs: Constructor does real work
New keyword in constructor or fieldsStatic method callsAnything more than field assignmentsObject not fully initialized after constructorControl flow
Smell: Digging into Collaborators
class TicketPriceCalculator {
MovieTheaterService service;
TicketPriceCalculator(MovieTheaterService service) {
this.service = service;
}
BigDecimal calculatePrice(User user, Invoice invoice) { UserBalance balance = user.getBalance();
BigDecimal amount = invoice.getSubtotal();
return service.calculateTotal(balance, amount);
}
}
Flavour: Digging around
class TicketPriceCalculator {
MovieTheaterService service;
TicketPriceCalculator(MovieTheaterService service) {
this.service = service;
}
BigDecimal calculatePrice(User user, Invoice invoice) { UserBalance balance = user.getBalance();
BigDecimal amount = invoice.getSubtotal();
return service.calculateTotal(balance, amount);
}
}
Flavour: Digging around
TicketPriceCalculator calculator; //SUT
@Mock MovieTheaterService service;
@Test void calculatePrice_tooMuchWork() { calculator = new TicketPriceCalculator(service);
given(user.getBalance()).willReturn(new DummyBalance()); given(invoice.getSubtotal()).willReturn(new BigDecimal("9.3"));
BigDecimal result = calculator.calculatePrice(balance, invoice);
assertThat(result, is(new BigDecimal("8.3"));}
Hard to test? Digging around
TicketPriceCalculator calculator; //SUT
@Mock MovieTheaterService service;
@Test void calculatePrice_tooMuchWork() { calculator = new TicketPriceCalculator(service);
given(user.getBalance()).willReturn(new DummyBalance()); given(invoice.getSubtotal()).willReturn(new BigDecimal("9.3"));
BigDecimal result = calculator.calculatePrice(balance, invoice);
assertThat(result, is(new BigDecimal("8.3"));}
Hard to test? Digging around
class TicketPriceCalculator {
MovieTheaterService service;
TicketPriceCalculator(MovieTheaterService service) {
this.service = service;
}
// Deceitful API, Law of Demeter
BigDecimal calculatePrice(User user, Invoice invoice) { UserBalance balance = user.getBalance(); BigDecimal amount = invoice.getSubtotal(); return service.calculateTotal(balance, amount);
}
}
Flavour: Digging around
class TicketPriceCalculator {
MovieTheaterService service;
TicketPriceCalculator(MovieTheaterService service) {
this.service = service;
}
// Problem? Imagine this in a big application, your code
// will be much more complicated than you (and your team) need BigDecimal calculatePrice(User user, Invoice invoice) { UserBalance balance = user.getBalance(); BigDecimal amount = invoice.getSubtotal(); return service.calculateTotal(balance, amount);
}
}
Flavour: Digging around
class TicketPriceCalculator {
MovieTheaterService service;
TicketPriceCalculator(MovieTheaterService service) {
this.service = service;
}
// Cleaner API, forget about "Middle men" class
BigDecimal calculatePrice(UserBalance balance, BigDecimal am) { return service.calculateTotal(balance, am);
}
}
Fix: Digging around
TicketPriceCalculator calculator; //SUT
@Mock MovieTheaterService service;
@Test void calculatePrice_tooMuchWork() { calculator = new TicketPriceCalculator(service);
UserBalance balance = new DummyBalance("1.0");
BigDecimal amount = new BigDecimal("9.3"));
BigDecimal result = calculator.calculatePrice(balance, amount);
assertThat(result, is(new BigDecimal("8.3"));}
Fix: Digging around
Digging into Collaborators, more flavours
class MovieTheaterService {
void processOrder(UserContext context) { User user = context.getUser();
Order order = context.getOrders().byUser(user).now();
order.process();
}
}
Flavour: "Context", "Container"...
class MovieTheaterService {
void processOrder(UserContext context) { User user = context.getUser(); Order order = context.getOrders().byUser(user).now(); order.process();
}
}
Flavour: "Context", "Container"...
class MovieTheaterService {
void processOrder(UserContext context) { User user = context.getUser(); Order order = context.getOrders().byUser(user).now(); order.process();
}
// Test? Create a UserContext with ton of mocks and stuff
}
Flavour: "Context", "Container"...
class MovieTheaterService {
void processOrder(UserContext context) { User user = context.getUser(); Order order = context.getOrders().byUser(user).now(); order.process();
}
// Test? Create a UserContext with ton of mocks and stuff
}
Flavour: "Context", "Container"...
class MovieTheaterService {
void processOrder(User user, Order order) { order.process();
}
// Test? Sure! You got everything you need in place.
}
Fix: "Context", "Container"...
Warning signs: Digging into Collaborators
Objects to use other objects, not themself. Having to create mocks that return other mocks
Law of Demeter everywhere Seeing more than one period (.) in a method
chaining. Going deep in the object graph: mgs.getSolid().findOcelot().now()
Exception: DSLs
Suspicious names Context, Environment, Container...
Smell: Global state and Singletons
@Test void chargeCreditCard() { // Example: @mhevery CreditCard cc = new CreditCard("9999 0000 7777", 5, 2009);
assertThat(cc.charge(30.0), is(true));}
Flavour: They are liars!
@Test void chargeCreditCard() { CreditCard cc = new CreditCard("9999 0000 7777", 5, 2009);
assertThat(cc.charge(30.0), is(true));
// Fails at runtime:
// java.lang.NullPointerExpection}
Flavour: They are liars!
@Test void chargeCreditCard() { CreditCardProcessor.init(); CreditCard cc = new CreditCard("9999 0000 7777", 5, 2009);
assertThat(cc.charge(30.0), is(true));}
Flavour: They are liars!
@Test void chargeCreditCard() { CreditCardProcessor.init(); CreditCard cc = new CreditCard("9999 0000 7777", 5, 2009);
assertThat(cc.charge(30.0), is(true));
// Argh! This is getting awkward
// java.lang.NullPointerExpection}
Flavour: They are liars!
@Test void chargeCreditCard() { CreditCardProcessor.init();
OfflineQueue.start(); CreditCard cc = new CreditCard("9999 0000 7777", 5, 2009);
assertThat(cc.charge(30.0), is(true));}
Flavour: They are liars!
@Test void chargeCreditCard() { CreditCardProcessor.init();
OfflineQueue.start(); CreditCard cc = new CreditCard("9999 0000 7777", 5, 2009);
assertThat(cc.charge(30.0), is(true));
// Are you kidding?
// java.lang.NullPointerExpection}
Flavour: They are liars!
@Test void chargeCreditCard() { Database.connect("hsqldb"); CreditCardProcessor.init();
OfflineQueue.start();
CreditCard cc = new CreditCard("9999 0000 7777", 5, 2009);
assertThat(cc.charge(30.0), is(true));}
Flavour: They are liars!
@Test void chargeCreditCard() { Database.connect("hsqldb"); // Global State CreditCardProcessor.init(); // Global State
OfflineQueue.start(); // Global State
CreditCard cc = new CreditCard("9999 0000 7777", 5, 2009);
assertThat(cc.charge(30.0), is(true));
// How do we know this? Sure it's documented somewhere... right?
}
Flavour: They are liars!
How do you provide global variables in languages without global variables?
"Don't. Your programs will thank you for taking the time to think about design instead."
-- Kent Beck, JUnit and TDD creator
Fix: They are liars!
Fix: They are liars!
Dependency Injection
Fix: They are liars!
Dependency Injection
Dependency Injection
Fix: They are liars!
Dependency Injection
Dependency Injection
Dependency Injection
Fix: They are liars!
Dependency Injection
Dependency Injection
Dependency Injection
Dependency Injection
Fix: They are liars!@Test void chargeCreditCard() { Database db = mock(Database.class); Queue queue = new OfflineQueue(db); CreditCardProcessor ccProc = new CreditCardProcessor(queue); CreditCard cc = new CreditCard(ccProc, "9999 0000 7777"); cc.charge(30.0);}
Fix: They are liars!@Test void chargeCreditCard() { Database db = mock(Database.class); Queue queue = new OfflineQueue(db); CreditCardProcessor ccProc = new CreditCardProcessor(queue); CreditCard cc = new CreditCard(ccProc, "9999 0000 7777"); cc.charge(30.0);}
Fix: They are liars!@Test void chargeCreditCard() { CreditCardProcessor ccProc = mock(CreditCardProcessor.class); CreditCard cc = new CreditCard(ccProc, "9999 0000 7777"); cc.charge(30.0);}
Singletons and JVM Singletons Gang of four, static instance field. Best known as an Anti-pattern.
But there's nothing wrong about having one instance of an Object Application Singleton
Managed by someone, eg: Guice @Singleton scope
About Singletons
Global state is Evil Dirty design, enables Action at a distance (anti-pattern), extremely-hard-to-test, etc.
Caveat: When it's ok? Primitive or immutable constant reference
static final String DEVFEST = "Devfest rocks" When information only travels one way org.slf4j.Logger // but hard to test
About Global State
Warning signs: Global State and Singletons
Using SingletonsStatic fields or static methods Don't confuse with helper or factory methods
Static initialization block class App {
static {
MutableObject object = Registry.find();
}
}
Service Locators
Smell: Class does too much
Smell: Class does too much, aka
Kitchen Sink
Smell: Class does too much, aka
Kitchen SinkDumping Ground
Smell: Class does too much, aka
Kitchen SinkDumping GroundThis Class does this AND that AND ...
Smell: Class does too much, aka
Kitchen SinkDumping GroundThis Class does this AND that AND ...God Class
Smell: Class does too much, aka
Kitchen SinkDumping GroundThis Class does this AND that AND ...God ClassQuijote
Smell: Class does too much, aka
Kitchen SinkDumping GroundThis Class does this AND that AND ...God ClassQuijote"You can look at everything except this Class"
Smell: Class does too muchinterface MovieTheater {
void open();
void addShowtime(Showtime showtime);
Set<Showtime> getShowtimes();
void purchaseTicket(Buyer buyer, Movie movie);
UserBalance findUserBalance(User balance);
void processOrder(Order order);
// ...}
Smell: Class does too muchinterface MovieTheater {
void open();
void addShowtime(Showtime showtime);
Set<Showtime> getShowtimes();
void purchaseTicket(Buyer buyer, Movie movie);
UserBalance findUserBalance(User balance);
void processOrder(Order order);
// ...}
Fix: Class does too muchinterface MovieTheater { void open();
void addShowtime(Showtime showtime);
Set<Showtime> getShowtimes();}
interface BillingService { void purchaseTicket(Buyer buyer, Movie movie);
UserBalance findUserBalance(User balance);
void processOrder(Order order);}
Warning signs: Class does too much
Descriptive names for this smellTiny scroll barMany fields, many methods Use Sonar and LCOM4 (Lack of Cohesion) to check this
Many collaboratorsJust don't getting the Class
Dependency Injection enables you to write testable code
Deep synergies between testable code and good design
-- Michael Feathers
Maintainable code
Much more Guice!
JSR330 javax.inject -- Google + Spring effort
Roboguice Android, @InjectView, @InjectResource
Dagger From Square, original Guice committers, built on JSR330, pretty exciting!
Writing testable code Miško Hevery
Working effectively with legacy code Michael Feathers
The testability Explorer blog Blog, Miško Hevery
Dependency Injection Book, Dhanji R. Prasanna
Test Driven - Practical TDD for Java devs Book, Lasse Koskela (2007)
Effective Unit Testing - For Java devs Book, Lasse Koskela (2012)
xUnit Test Patterns Book, Gerard Meszaros
Painless Testing Slides, in Java or Python. Johannes Seitz
References
Q & A
[email protected]@jordi9