Instantly share code, notes, and snippets.

Embed
What would you like to do?
Interfaces for presenters in MVP are a waste of time!

##Interfaces for presenters in MVP are a waste of time!

It's been a long time since we started talking about MVP. Today, the discussion is about if creating an interface for the Presenter in MVP is needed.

This is the Model View Presenter pattern's schema:

MVP Schema

In this schema the Model box is related to all the code needed to implement your business logic, the presenter is the class implementing the presentation logic and the view is an interface created to abstract the view implementation.

Why the view should be implemented with an interface in this pattern? Because we want to decouple the code from the view implementation. We want to abstract the framework used to write our presentation layer independently of any external dependency. We want to be able to change the view implementation easily if needed. We want to follow the dependency rule and to improve unit testability. Remember that to follow the dependency rule high level concepts like the presenter implementation can't depend on low level details like the view implementation.

Why the interface is needed to improve unit testability? Because to write a unit test all the code should be related to your domain and no external systems as a SDK or a framework.

Let's go to put an example related to a login screen implemented for Android:

/**
* Login use case. Given an email and password executes the login process.
*/
public class Login {

  private LoginService loginService;
  
  public Login(LoginService loginService) {
    this.loginService = loginService;
  }
  
  public void performLogin(String email, String password, LoginCallback callback) {
  	boolean loginSuccess = loginService.performLogin(email, password);
  	if (loginSuccess) {
  	  callback.onLoginSuccess();
  	} else {
  	  callback.onLoginError();
  	}
  }
}

/**
* LoginPresenter, where the presentation logic related to the login user interface is implemented.
*/
public class LoginPresenter {
	
	private LoginView view;
	private Login login;
	
	public LoginPresenter(LoginView view, Login login) {
		this.view = view;
		this.login = login;
	}
	
	public void onLoginButtonPressed(String email, String password) {
		if (!areUserCredentialsValid(email, password)) {
			view.showInvalidCredentialsMessage();
			return;
		}
	
		login.performLogin(email, password, new LoginCallback {
			void onLoginSuccess() {
				view.showLoginSuccessMessage();
			}
			
			void onLoginError() {
				view.showNetworkErrorMessage();
			}
		});
	}
	
}

/**
* Declares what the presenter can do with the view without generating coupling to the view implementation details.
*/
public interface LoginView {

  void showLoginSuccessMessage()
  void showInvalidCredentialsMessage()
  void showNetworkErrorMessage()

}

public class LoginActivity extends Activity implements LoginView {

  .........

}

Please don't pay attention to the code syntax. I've written this from the scratch and it's almost pseudocode.

Why the View interface is needed here? To be able to write a unit test replacing the view implementation with a test double. Why is this needed in the unit test context? Because you don't want to mock the Android SDK and use the LoginActivity inside your unit tests. Remember that if you write a tets where the Android SDK is part of the SUT this is not a unit test.

At this part of the implementation is clear. We need an interface to do not depend on the view implementation.

Some developers have decided to add also an interface in top of the presenter. If we follow the previous example the implementation could be like this:

public interface LoginPresenter {

  void onLoginButtonPressed(String email, String password);
}

public class LoginPresenterImpl implements LoginPresenter {
  ....
}

or

public interface ILoginPresenter {

  void onLoginButtonPressed(String email, String password);
}

public class LoginPresenter implements ILoginPresenter {
  ....
}

What's the problem with this extra interface? IMHO this interface is not needed and is just adding complexity and noise to the development. Why?

  • Look at the class name. When the interface is not needed the names used become weird and don't add semantic to the code.
  • That interface is the class we have to modify to add a new method when the presentation logic has a new path and then we have to also update the implementation. Even when we use modern IDEs this is a waste of time.
  • The navigation in the code could be difficult to follow because when you are inside the Activity (the view implementation) and you want to navigate to the presenter the file where you are going to go next is the interface when most of the time you want to go to the implementation.
  • The interface is not improving the project testability. The presenter class can be easily replaced with a test double using any mocking library or any hand made test doubles. We don't want to write a test using the activity as SUT and replacing the presenter with a test double.

So...what is the LoginPresenter interface adding here? Just noise :)

But.....when should we use an interface? Interfaces should be used when we have more than one implementation (In this case the presenter implementation is just one) or if we need to create a strong boundary between our code and a third party component like a framewokr or a SDK. Even without interfaces we could use composition to generate abstraction, but the usage of an interface in Java is easier :) So, if you have more than one implementation of the same thing or you want to generate a strong boundary, then, add an interface. If not.....do not add more code. Remember, the less code to maintain the better. Remember that the usage of interfaces is not the only way to decouple our code and generate abstraction

But...what if I want to decouple the view implementation from the presenter implementation? You don't need to do that. The view implementation is a low level detail and the presenter imlementation is a high level abstraction. Implementation details can be coupled to high level abstractions. You want to abstract your domain model from the framework where it's executed, but you don't want to abstract in the opposite direction. Trying to reduce the coupling between the view implementation and the presenter is just a waste of time.

I've created this gist to discuss about this topic, please feel free to add any comment using code examples if needed :)

Extra ball: If you are thinking in different testing strategies for Android and the presentation layer I wouldn't use a unit test to test the presentation logic replacing the view with a test double. I'd try to use an approach like the one described here where the SUT is the whole presentation layer and not just the presenter (the test doubles are used to replace the use case).

@pedrovgs

This comment has been minimized.

Owner

pedrovgs commented May 11, 2016

I'm sure @delr3ves has something to say here :)

@davideme

This comment has been minimized.

davideme commented May 11, 2016

Reference: The Model-View-Presenter (MVP) Pattern

The presenter is an abstraction on your view implementation, is not aware of which framework the view will use. The presenter contains the logic to respond to the events and alters the state of the view. There is no general best practice using this pattern in using the presenter behind an interface only an overhead for the programmer.

@pedrovgs

This comment has been minimized.

Owner

pedrovgs commented May 11, 2016

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

A ver por el principio, respeto profundamente todos los acuerdos a los que un equipo llegue internamente, por lo que si esta forma de trabajar os va bien, y la experiencia así os lo ratifica perfecto. Pero mi experiencia ha sido otra y me he encontrado en multitud de ocasiones código basura precisamente por no poner cuidado en estas cosas.

Tomando como ejemplo el presenter, para mi (y esto es muy personal) es vital que este este abstraido, básicamente porque a mis consumidores les importa un mojon como haga yo las cosas, solo les interesa saber cuales son mis puntos de entrada y que espero de ellos.

Respecto a los nombres, dejarme enseñaros una implementación.

public interface LoginPresenter {
  void onLoginButtonPressed(@NonNull String email, @NonNull String password);

  interface View {
    void showLoginSuccessMessage();
    void showInvalidCredentialsMessage();
    void showNetworkErrorMessage();
  }
}

Este presenter me permite ofrecer a mis compañeros el contrato con en el que vamos a trabajar, sin necesidad de generar código fake, y controlar los flujos de información ya que el resto de métodos de la/las implementación/es de este contrato han de ser privados si o si.

public class DefaultLoginPresenter implements LoginPresenter {
  private final View view;

  @Inject
  public DefaultLoginPresenter(DefaultLoginPresenter.View view) {
    this.view = view;
  }

  @Override
  public void onLoginButtonPressed(@NonNull String email, @NonNull String password) {
    //Do something.
  }
}

La implementación del presenter es lo de menos, ya que nadie, ningún componente de mi arquitectura la conocerá a excepción de mis módulos de inyección. Solo estos sabrán de la existencia de estas clases.

public class LoginActivity extends Activity implements LoginPresenter.View {
  @Inject LoginPresenter presenter;

  @Override public void showLoginSuccessMessage() {
    //Do Something
  }

  @Override public void showInvalidCredentialsMessage() {
    //Do Something
  }

  @Override public void showNetworkErrorMessage() {
    //Do Something
  }
}

La implementación de la vista, nada sabe de la implementación del presenter, solo conoce que puntos de entrada tiene.

En resumen, no entiendo el porque esta forma de trabajar empeora la legibilidad, no veo que sea un YAGNI ya que se usan todos los componentes y me permite trabajar más seguro con el resto del equipo frente a exponer métodos que no están consensuados.

@alorma

This comment has been minimized.

alorma commented May 11, 2016

Totalmente de acuerdo con @txusballerteros, al tener el presenter abstraido de esta manera puedo preparar la UI / "Backend" por separado, cosa que facilita el trabajo en equipo

Presenter abstraido = API / contrato

@davideme

This comment has been minimized.

davideme commented May 11, 2016

Remember: “Code is read much more often than it is written.”
Even if it's easier to write interfaces to all dependencies for the initial developer, the code will be easier to maintain if you only use interfaces when needed.

@pedrovgs

This comment has been minimized.

Owner

pedrovgs commented May 11, 2016

@txusballesteros, solo tienes una implementación del presenter y te da exactamente igual que tu activity conozca la implementación de tu presenter. Es abstraer por abstraer, que es lo que quería deciros en este "gist/blog post". No estás añadiendo ningún valor al código poniendo una interfaz ahí. Mira a los smells, como el nombre del presenter DefaultLoginPresenter ¿qué quiere decir default en ese contexto si no tienes más implementaciones? O que cuando añades comportamiento al presenter siempre tienes que cambiar la interfaz y la implementación. O que cuando navegas de la implementación de la vista al presenter tienes que ir primero a la interfaz y luego a la única implementación. Para mi, estás escribiendo más código del que te hace falta y por tanto añadiendo solo ruido :/

Si seguimos esta regla de "mi componente no tiene por qué saber como otro haga las cosas" vamos a poner interfaces delante de todas las clases? Hay otros mecanismos de los lenguajes como los modificadores de visibilidad o patrones creacionales como factorías o builders que te pueden ayudar a conseguir eso.

@pedrovgs

This comment has been minimized.

Owner

pedrovgs commented May 11, 2016

@alorma el uso de interfaces no es la única forma de generar abstracción. Como decía a @txusballesteros el uso de patrones creacionales y modificadores de visibilidad nos pueden ayudar a ocultar detalles de implementación. Si el uso de interfaces fuera el único mecanismo que tenemos para generar abstracción y ocultar detalles de implementación qué hacemos en lenguajes como python donde no existen las interfaces?

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

También es cierto que comparto el punto de vista de pensar antes de abstraer, por ejemplo, si estoy usando un patrón command o similar, estoy casi convencido de que ese comando si que tendrá un contrato común con sus hermanos, pero sus consumidores usaran la implementación ya que se a ciencia cierta que por su naturaleza no va a mutar a otra implementación.

Pero en los componentes básicos de la arquitectura y en los que los equipos han de trabajar en cooperación. Tengo muy claro que la abstracción coherente y sencilla es necesaria y buena.

Si ya entramos en tiempos de desarrollo, esto ya es un tema más peliagudo, ya que dependerá mucho del tipo de empresa en la que trabajes. No es lo mismo trabaja en producto, en el que tratas de tener el código preparado para los cambios, porque sabes que los vas a tener, que trabajar en consultoría en los que el tiempo que consumas en hacer el trabajo es parte fundamental de tu rentabilidad. Y por esta estás dispuesto no a sacrificar ciertas prácticas, sino a ser más tolerante y dejar de lado cosas que en tu escenario son accesorias porque sabes que no van a cambiar.

@pedrovgs

This comment has been minimized.

Owner

pedrovgs commented May 11, 2016

wtf?

perdona que me sorprenda, pero la conversación acaba de cambiar completamente de tema y no me lo esperaba

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@pedrovgs comparto tu punto de vista sobre la legibilidad del código per no al 100%, como bien dices el DefaultXXXX no me dice nada en concreto, pero es que tampoco quiero saberlo, salvo que es un acuerdo interno de mi equipo que las implementaciones se llaman así, por lo que podría encontrarla sin problema.

Respecto a las modificaciones, es cierto, he de modificar el contrato en la interface y después implementarlo en la clase, y también es cierto que cada vez que tengo que hacerlo sudo de lo costoso que es (perdón por la ironía).

Si me centro en los modificadores de acceso para ocultar lo que es público de lo que no, estoy delegando al equipo completo que sean coherentes con lo que hacen, y yo les quiero mucho, pero todos somos humanos, me resulta más sencillo de revisar en una PR, que si un método no es @Overrride, no puede ser público.

Lo que no te dire es la que tuve que montar la última vez que tuve que hacer un Split de un presenter a dos implementaciones diferentes, y por no estar abstraido el refactor me llevo casi dos días. Sin contar el tiempo que perdió el QA para probar los componentes que tuve que migrar por fuerza al crear la segunda implementación. Os digo esto porque si queremos ser prácticos esto es importante.

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@pedrovgs no quería desviar el foco del debate, pero es que creo que también es importante porque ambos escenario son interesantes de analizar.

@FranRiadigos

This comment has been minimized.

FranRiadigos commented May 11, 2016

Hola, sólo por curiosidad me gustaría dar mi opinión. Quizá si sea a veces redundante crear interfaces para los presenters, pero para mi realmente son contratos que ha de seguir tu equipo y que obliga a que se definan en algún sitio. De todos modos, mi pregunta es, ¿no sería interesante utilizar esta abstracción en interfaces para reutilizar las operación de elementos (presenters) internos o de otra implementación, en otros presenters?
Mi ejemplo es el siguiente:
Si tenemos un listado de items, el cuál tiene su propio presenter que carga estos items, y estos items tienen su propio presenter con operaciones como dismiss y marcar como favorito, pero "como usuario quiero poder hacer dismiss o marcar como favorito un item directamente en el listado", ya que estas dos operaciones realmente pertenecen a un contrato de los items, si abstraigo estas funcionalidades, ¿no veis útil que así pueda extender estas operaciones de los items dentro del presenter del listado?
Así mi listado tendría su propio contrato de operaciones, y añadiría funcionalidades de elementos internos.
De hecho la interfaz de un presenter puede heredar de otras interfaces más sencillas destinadas a unas operaciones en concreto que son repetitivas.
¿Como lo veis?

@davideme

This comment has been minimized.

davideme commented May 11, 2016

No estamos diciendo que no tenemos que usar interfaces para abstraer componentes. Si no que en el caso del MVP, el presenter generalmente solo tiene 1 implementación. Y usar una class LoginPresenter con metodos vacios no es una mala primera iteración. Nos quitamos de la parte DefaultLoginPresenter, no tengo que mirar como esta configurado el injector para entender el codigo de mi vista.

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@davideme si solo tienes una implementación no has de mirar tu injector para nada. Solo hay una opción.

@davideme

This comment has been minimized.

davideme commented May 11, 2016

@kuassivi si tiene sentido este caso para ofrece una lista de funcionalidades comunes a los presenter, pero seguirias referenciando la implementacion concreta en al vista.

@Serchinastico

This comment has been minimized.

Serchinastico commented May 11, 2016

Si me centro en los modificadores de acceso para ocultar lo que es público de lo que no...

Para eso es para lo que sirven: https://en.wikipedia.org/wiki/Object-oriented_programming#Encapsulation

@pedrovgs

This comment has been minimized.

Owner

pedrovgs commented May 11, 2016

🔝

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@kuassivi a tu planteamiento, en mi caso suele ser habitual, si una vista tiene comportamientos, que tenga un presenter para controlarlos. Con esto quiero decir que si tienes un Fragment con una lista esta tendrá su presenter. Y si ese listado por ejemplo tiene una CutomView con comportamientos, esta vista podría tener si propio presentar para controlarla.

Lo de que un presenter herede de otro, ya me da un poco más de miedo, en ese caso suelo preferir componer. Y el approach que te comento de la vista, seria una especie de composición (y ya se que no lo es).

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@Serchinastico nadie ha dicho lo contrario, de otra manera no existirían.

@gragera

This comment has been minimized.

gragera commented May 11, 2016

Los métodos públicos de una implementación de un Presenter constituyen un contrato al igual que los métodos de una interfaz. Si sabes a ciencia cierta que sólo va a haber una implementación de ese contrato y sabes que no vas a necesitar sustituir la implementación, la interfaz no te aporta nada, es redundante y sólo aporta ruido :)

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@gragera el problema es que yo no soy adivino y no se que es lo que mi product owner me tiene preparado. Por lo que ya voy sentando las bases para poder aceptar cambios sin que afecten al código que ya ha sido testado y validado.

Por otro lado, claro que puedes usar los modificadores, es más has de usarlos, pero como ya he dicho, todos nos podemos equivocar y dejar una clase exponiendo métodos que no son públicos. Con una contrato de por medie es más fácil localizar estos smells en una Code Review y eliminarlos.

Con todo esto no quiero decir que abstraer una clase que hoy solo tiene una implementación sea bueno o malo, cada uno sabrá que es lo que más le conviene en un momento dado.

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

Ya he leído varias veces que usar la abstracción en un presenter empeora la legibilidad ya que para navegar desde la vista hasta la implementación del presenter has de pasar primero con la interface. Y si, es cierto. Pero también es cierto que si desde mi vista tengo que mirar el código del presenter, quizás es que el código de mi vista no este muy bien formado y no sea lo suficientemente legible. Con esto quiero decir, que si veo una llamada a mi presenter y con esta no me queda claro que está haciendo, mal vamos.

@gragera

This comment has been minimized.

gragera commented May 11, 2016

el problema es que yo no soy adivino y no se que es lo que mi product owner me tiene preparado. Por lo que ya voy sentando las bases para poder aceptar cambios sin que afecten al código que ya ha sido testado y validado."

Esto está muy bien, pero no tiene nada que ver con lo que estamos hablando. Si tienes que cambiar la API pública de tu presenter, vas a tener que cambiar la interfaz y la implementación, mientras que en el otro caso sólo tienes que cambiar la implementación. De nuevo la interfaz no aporta nada.

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

Aporta desde el punto de vista cuando tengo que reutilizar un presenter y no conozco cual es su contrato, me resulta mucho más sencillo de entender como funciona y como he de usarlo si solo tengo que ver sus métodos públicos y no tengo que ver sus 300 líneas de código.

Y si, vas a perder 1seg. de tu vida diciéndole a Android Studio que te genere el método.

@gragera

This comment has been minimized.

gragera commented May 11, 2016

No estamos hablando de tools (Xcode por ejemplo tiene un bonito botón de view generated interface que te muestra sólo los métodos públicos) sino de ingeniería del software. Y en este caso, creo que la interfaz es una abstracción redundante.

@flipper83

This comment has been minimized.

flipper83 commented May 11, 2016

vayamos por partes, porque me parece un discusión un poco absurda. Hablamos de que utilizamos un interfaz para crear un contrato, que que queremos que varias clases implementen, para poder depender de una abstracción y no de la implementación concreta, espero que hasta aquí estemos todos de acuerdo.

Luego una de las reglas del buen diseño de software dice: Has few elements. Por lo cual si tenemos solo una implementación del contrato podemos destacar que no hace falta tener un interfaz con con el contrato porque está explícito en la definción de la clase a traves de sus métodos públicos.

En el hipotético caso que te haga falta una segunda implementación de esa clase con código distinto, pues nada más fácil que extraer el interfaz y depender de esta nueva abstracción, para los que os mole AndroidStudio, os lo hace con un click ;)

Estas reglas no son solo para los presenters, si no para la construcción de cualquier software. No escribias código de más. Hay que escribir el mínimo código que esté bien y responda a nuestro problema. Claramente en el caso de un presenter que solo tiene una implementación, no tiene ningún sentido crear un interfaz. Si me decís que lo haces por motivos de testing, recordar que debemos evitar a menos que sea imposible hacerlo, crear código solo para tests ;).

Esa es mi opinión y espero haber echado luz al asunto.

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@flipper83 a ti jamás te llevaría la contraria, pero sigo opinando lo mismo. Depender de una concreción me parece un smell. Pero es una opinión muy personal.

@colymore

This comment has been minimized.

colymore commented May 11, 2016

Menos código, menos bugs, asi de simple.
Estamos acostumbrados a crear interfaces para una unica implementacion y asi creo que no estas abstrayendo de nada a menos que ocultes métodos públicos que tenga la implementación concreta bajo la interfaz. Por lo tanto, para mi, si añade complejidad que no aporta nada.

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@colymore menos código, mismos bugs, no he visto nunca un bug en una interface. Y menos código supone también ser menos modular y en consecuencia más recio a la hora de asumir cambios.

@flipper83

This comment has been minimized.

flipper83 commented May 11, 2016

Tu lo has dicho te parece un smell, los smell de código no es código que este mal es código que quieres ver porque está hecho así, al igual que para mi cualquier interfaz con una sola implementación me parece un smell.

Si ves mis primeras implementaciones de clean, están ahí bien a la vista tenia un montón de interfaces, que con el tiempo me di cuenta que eran redundantes y no aportaban nada. Espero que con el tiempo e iteraciones veas lo que te decimos.

Y como bien ha dicho @colymore menos código, menos bugs, y agregaría menos tiempo para leer, menos tiempo para mantener.

Lo que haces tiene un nombre en TDD que es abstración prematura, como tienes miedo a que tu software mute no esperas a que pase y lo haces antes, y es un problema, porque como solo hay una implementación es muy posible que el contrato esté ligado a la implementación del mismo. Al final parece que estás programando en c, con el .h y el .cpp. Pero bueno lo dejo aqui, porque no quiero dar más vueltas sobre lo mismo. El grano de discusión es demasiado fino y concreto, que ya está todo dicho.

@enderscode

This comment has been minimized.

enderscode commented May 11, 2016

Me baso exactamente en lo que ha dicho Flipper, pero también quiero dejar mi granito de arena. Tengo que decir que la verdad, ni siquiera se me había pasado por la cabeza la posibilidad de abstraer el Presenter por un simple hecho de direccionalidad.

Es la Vista la que se encarga de instanciar el presenter y ese presenter sólo va a ser utilizado por esa vista. Cuando trabajas desde el punto de vista del presenter no sabes quién va a ser el que te instancie y dejas tus necesidades bajo el check de "Cuando me crees, cumple este contrato".

Al ser la vista la que instancia al presenter, meter una abstracción ahí sería tan útil como crear un ArrayList como List (List loquesea = new ArrayList<>();). Usando una abstracción más restrictiva para un objeto instanciado que controlas completamente. Ningún beneficio, y sin embargo ocultas su implementación.

@JorgeCastilloPrz

This comment has been minimized.

JorgeCastilloPrz commented May 11, 2016

Llego muy tarde, pero me da igual, yo pongo mi comment!

Disclaimer: Esto lo digo a toro pasado, con lo cuál no tengo mérito ninguno chavales, peeeero ahí voy igualmente! 💃 😄

image

Bajo mi punto de vista, en el caso de Java (no tengo intención de entrar en otros lenguajes), una interfaz existe para abstraer un comportamiento / contrato que vas a necesitar implementar por diferentes clases. Por tanto considero que pierde su valor si partimos de una implementación que siempre va a ser única. Aquí podríamos pensar que ojo, ¿y si mi implementación es única en el código de producción pero tengo una segunda implementación fake de cara a los test? Honestamente, yo sacaría esa implementación de esta disusión, y más abajo explicaré por qué. Así que para sintetizar, usaría una interfaz si los requisitos de mi producto me están pidiendo varias implementaciones diferentes a lo largo del código de producción de mi aplicación.

Sample: Imaginemos un par de implementaciones diferentes del mismo presenter porque mi view es capaz de funcionar bajo 2 modos diferentes de presentación. Un ejemplo que hice yo mismo en JT: PostJobPresenter gestiona la lógica de presentación para postear un nuevo job, pero tiene 2 modos de uso. Puede servir tanto para editar un job ya existente como para postear uno nuevo, con los cambios de comportamiento en la vista y el dominio que eso supone. (Que los hay). De esta manera evito tener un súper presenter con muchas líneas que sepa manejar ambas situaciones, y lo que hago es switchear dos implementaciones diferentes dependiendo de si el tío viene navegando desde un botón de edición o desde el FAB para publicar un job nuevo.

Inciso de bullshit: Mucha gente tiene su punto de vista algo viciado porque piensa que con Dagger o cualquier otro framework de DI sólo podemos inyectar dependencias partiendo de interfaces, lo cuál sabemos que no es cierto.

Más cositas: El caso de empresas (como el que describe @txusballesteros) que necesitan trabajar en paralelo en la vista y el presenter es necesario establecer un "contrato" a seguir que sea común, y es cierto que para ello son útiles. Pero como bien explicaba @davideme, puedes lograrlo creando una implementación early de tu clase concreta, en este caso del presenter, trabajando ambas partes a partir de ese nexo. Habiendo pensado un poco sobre el tema, y tras haber discutido esta forma de trabajar más de una vez en mi empresa, yo prefiero este approach en lugar de meter sin más una interfaz, que al final es una clase adicional no necesaria dentro de tu código.

Y para terminar, como decía antes, yo intentaría mantener el tema testing un poco al margen de usar o no interfaces, porque al fin y al cabo para mí tiene poco que ver. Lo que marca el hecho de poder aislar el SUT e introducir test doubles en los huecos abiertos para sus colaboradores, es que sus dependencias estén inyectadas (independientemente de si son abstractas o directamente concreciones, y si están inyectadas a partir del constructor, setters, fields, o con un framework que te inyecta código en tus .class rollo lombok y te resuelve las dependencias en tiempo de compilación). De este modo, podrás inyectarle en tiempo de testing una implementación / extensión diferente. Recordad que los test doubles se pueden crear por extensión, ya que nuestras clases concretas también tienen un contrato expuesto hacia fuera con sus métodos y fields públicos (incluso package visibility), con lo que siempre podrás overridearlos y alterar su comportamiento. Por tanto, si yo tuviera que mockear mi presenter (aunque no sea una práctica común si testeas la vista, ya que acaba siendo un test muy end to end) lo que haría sería inyectar en tiempo de testing una extensión del mismo redefininendo los métodos públicos que son los que exponen hacia la vista el contrato de uso.

El punto de vista de la encapsulación de infromación, exposición, visibilidad etc, es muy importante cuando creas librerías o cualquier tipo de código para terceros y tienes que tener en cuenta la facilidad de uso de puertas para fuera.

@delr3ves

This comment has been minimized.

delr3ves commented May 11, 2016

Estoy con @flipper83 en eso de que esto no es una conversación que aplique solo para los presenters. El hacer interfaces o no, creo que es una cuestión de gustos más que otra cosa. Está claro que hay momentos en las que aportan muchísimo valor y creo que nadie lo cuestiona (multiples implementaciones) pero para los casos en los que el valor es más dudoso, se me ocurren argumentos a favor y en contra de todos los puntos que se están comentando:

- Sobre la legibilidad
Tener la interfaz mejora la legibilidad, siempre y cuando los nombres de tus métodos estén bien elegidos, en el sentido de que ves todo lo importante del tirón y no tienes el ruido (los detalles de implementación) añadido de tener más código que el del propio contrato. Eso claro, si lo que quieres ver es que hace tu presenter (por centrarnos en el ejemplo) a grandes rasgos. Si solo te interesa ver lo que hace un método en concreto, la empeoran en el sentido de que estás a un click o atajo de teclado más de ver los detalles de implementación, pero no me parece una razón de peso para descartarlas.

- Sobre testabilidad
Si quieres aislar tu SUT, dependiendo del framework de mocking que utilices, el tener una implementación o una interfaz puede ser o no ser un problema (ya no debería serlo). Otra cosa es que no quieras aislar cieras partes de tu sistema y la interfaz no te aporte demasiado.

- Sobre las consideraciones a la hora de diseñar
Aquí rompo una lanza en favor de las interfaces, pero tampoco me parece una razón de peso para hacerlas. En la práctica, yo (Sergio Arroyo) cuando estoy definiendo una interfaz, dedico más tiempo a pensar en el método, en qué tiene que hacer y no cómo tiene que hacerlo. Me pongo el sombrero de pedir y me marco unos requisitos para definir mi contrato. Que sea intuitivo, que me de pistas de como acabó el resultado (devolviendo lo que tiene que devolver o lanzando lo que tenga que lanzar). Sé que no debería ser así, pero muchas veces cuando estoy implementando algo y me hace falta un método y lo tengo más a mano, como que no lo pienso tanto. Es un problema mío que no tiene por qué aplicar al resto del mundo.
Así que eso, yo considero la interfaz un lugar "sagrado" en el que meter solo lo justo y necesario y tener solo los métodos de un vistazo me gusta y me ayuda. Si bien es cierto, que eso también lo puedo tener viendo la estrucutra de la clase en cualquier IDE que se precie.

En resumen, me parece que no es una decisión tan sumamente relevante. Es cuestión de gustos, y lo que es mejor... es una decisión fácilmente revocable, al menos en teoría. Si te pasa como a @txusballesteros, eso de tener que hacer un

... Split de un presenter a dos implementaciones diferentes,...

el problema not tiene pinta de que fuera por hacer interfaz o no hacerla, sino que teniais una clase con muchas responsabilidades. Habría que ver si eso se hubiera detectado antes con la interfaz o sin ella. A priori colocar la interfaz en medio debería ser un proceso completamente mecánico, incluso automatizable (En Intellij: refactor -> extract -> interface).

Luego también hay que tener muy en cuenta el entorno de trabajo. No es lo mismo currar con gente super-senior que entiende profundamente todos los conceptos y tiene una capacidad bestial de refactorizar que currar en un equipo gigante con rotación, donde no todo el mundo tiene la capacidad de entender las decisiones y se busca la homogeneidad sobre todas las cosas y es más fácil hacer una norma que sea "para todas las clases que no sean contenedores de datos, hay que hacer una interfaz". Este segundo entorno no es el ideal pero no todo el mundo está en posición de elegir.

Y dejando de filosofar y hablando de lo que yo hago:

Antes hacía interfaces para todo, con el tiempo he dejado de hacerlas para muchas cosas, bien porque en mi equipo nadie las hace o bien porque no les veo valor en ese momento.

Ala, menudo chorizo os he soltado para no decir nada :P

@ffgiraldez

This comment has been minimized.

ffgiraldez commented May 11, 2016

Yo me he encontrado con el mismo caso que @txusballesteros de tener que implementar en paralelo, y lo resolví sin interfaces, pero en mi caso es porque mi Vista y su implementación ni siquiera conocen al Presenter siguiendo con el ejemplo de @pedrovgs

public interface LoginView {
  void initialize(LoginView.Delegate delegate);
  void showLoginSuccessMessage()
  void showInvalidCredentialsMessage()
  void showNetworkErrorMessage()
  public interface Delegate {
    void onLoginButtonPressed();
  }
}

public class LoginPresenter {
  private LoginView view;
  private final Login loginUseCase;
  private LoginView.Delegate viewDelegate = new LoginView.Delegate() {
    @Override
    public void onLoginButtonPressed() {
       loginUseCase.execute();
    }
  }

  public void initialize(LoginView view) {
   view.initialize(viewDelegate);
  }

}

Me gusta separar las cosas, aunque puede que sea un poco extremo, pero no me gusta que la implementación de la vista conozca los métodos del presenter para manejar los ciclos de vida.

La vista tal como yo la veo tiene dos flujos de dentro -> fuera y de dentro <- fuera, de dentro a fuera no es mas que agrupar varios Command en una clase y podemos en una primer commit acordar esos flujos para que no nos toquemos. permitiendo definir un contrato basado en la visibilidad de las clases

@pedrovgs

This comment has been minimized.

Owner

pedrovgs commented May 11, 2016

Gracias a todos por vuestras opiniones :) Las valoro mucho, que lo sepáis :) Da gusto ver como hay puntos de vista diferentes y los compartimos :)

@JorgeCastilloPrz

This comment has been minimized.

JorgeCastilloPrz commented May 11, 2016

La que has liao pollito

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

@pedrovgs A esto se le llama comunidad, pese a que me hayan caído ostias por todos lados. Y lo subscribo, gracias a @todos.

@txusballesteros

This comment has been minimized.

txusballesteros commented May 11, 2016

PDT: Que sepáis que pese a mi postura al respecto, entiendo perfectamente las motivaciones de los chicos de @Karumi para hacer las cosas como las hacen.

@cesards

This comment has been minimized.

cesards commented May 12, 2016

IMHO, and TL;DR I completely agree to all points mentioned at the beginning

Look at the class name. When the interface is not needed the names used become weird and don't add semantic to the code.

Correct, and Paul Blundell was talking about this a few days ago.

That interface is the class we have to modify to add a new method when the presentation logic has a new path and then we have to also update the implementation. Even when we use modern IDEs this is a waste of time.

I can imagine, saying "when the presentation logic has a new path" you mean you need to create new contracts/methods because the lifecycle of the activity makes you do that (onViewCreated()/onViewDestroyed/etc.). Since a while ago I try to divide presentation logic related to lifecycle, that way I don't mess up with my pure reactive implementation with view details. Here you are a simple example in order to have a clearer idea what I mean:

For the sample I'll use a reference from the SoundCloud guys repo

public class MyActivity extends LightCycleAppCompatActivity<MyActivity> implements View {
    @LightCycle LoginPresenterController controller;
    LoginPresenter loginPresenter;

    @Override
    protected void setActivityContentView() {
        setContentView(R.layout.main);

        loginPresenter = Application.getInstance().getPresenterModule(bla);
        presenterController = new PresenterController(loginPresenter);
    }

    // I try to think in a reactive way, so most of my views return observables handled by the presenter
    public Observable<Void> onRetryButtonClicked() {
        return RxView.clicks(retryButton)
    }

    //...
}

So this way I handle all my preseter details related to the lifecycle in a modular way:

public class PresenterController extends DefaultActivityLightCycle<MyActivity> {

   private final Presenter presenter;  

   public PresenterController(Presenter presenter) {
        this.presenter = presenter;
   }

    @Override
    public void onPause(MyActivity activity) {
        presenter.onViewWillHide();
    }

    @Override
    public void onResume(MyActivity activity) {
        presenter.onViewWillShow()
    }
}

and finally, my presenter handles all the subscriptions and do whatever it needs to do

@Override public void onViewWillShow() {
    super.onViewWillShow();

    loadUrl(signInUrl);

   addToAutoUnsubscribe(loginClicks.subscribe(view::selectAvatar));

   addToAutoUnsubscribe(view.onSelectionConfirmed()
      .doOnNext(ignored -> view.updateWhateverNeeded())
      .switchMap(this::updateUser)
      .observeOn(observeOn)
      .subscribe(ignored -> view.showSuccessfulUserUpdate()));
}

@Override public void onViewWillHide() {
    super.onViewWillHide();
    view.stopLoading();

    clearSubscriptions();
}

Do you get more or less my idea?

The navigation in the code could be difficult to follow because when you are inside the Activity (the view implementation) and you want to navigate to the presenter the file where you are going to go next is the interface when most of the time you want to go to the implementation.

Correct. It's easier creating a method directly in the implementation using the refactor tool:

@Override protected void onStart() {
  super.onStart();

  presenter.onViewWillShow(); // refactor (create) this method directly in the presenter
}

@Override protected void onStop() {
  presenter.onViewWillHide();

  super.onStop(); // refactor (create) this method directly in the presenter
}

The interface is not improving the project testability. The presenter class can be easily replaced with a test double using any mocking library or any hand made test doubles. We don't want to write a test using the activity as SUT and replacing the presenter with a test double.

TL;DR

YES

But.....when should we use an interface? Interfaces should be used when we have more than one implementation (In this case the presenter implementation is just one)

As I said before, I recommend all of you reading this article. Very instructive!!

@jmsalcido

This comment has been minimized.

jmsalcido commented Oct 20, 2016

I know that I am late but I want to give my opinion because this is the internet.

I totally disagree. I dont get the "confusing" part of an interface, it is a contract, an abstraction, even interface code as a POJO (talking about Java) can be transpiled to any language, it is pretty simple, there is no internal knowledge, can be used as a POXX (POCO, POJO, PONSO, even a C struct, whatever domain model or "plain old X object")

That's the magic of the interfaces, not "being able to mock them", for the mocking framework it is the same if it is a class or an interface or even an enum (if you are on Java).

Of course you will probably not get a second implementation of a Presenter or View, or any type of class, Service, Repository, or any fancy name for your objects here living in the same runtime as other, but you can still create a _StubPresenter_ in order to return real values to your application, this lovely _StubPresenter_ can be returned in your UI testing instead of the real implementation, just use the Stub and develop quickly and have faster tests instead of hitting your real implementation (flaky tests, anyone?).

I remember now that I answered over the article discussion: http://blog.karumi.com/interfaces-for-presenters-in-mvp-are-a-waste-of-time/

back in the day of publication, this was my original answer:

The advantages of having the interface is that you can have your StubImplementation and always inject that in your tests, you can at least have 2 implementations (one the production and other is the test).

It can be seen as "noise" but it is actually a good understanding of the SOLID (D) principle, depend on the contracts, not the concrete classes, it might be only 1 implementation on the actual codebase, but in the future it could be a different one, also, it is just a simple interface... I would include the interface, but still it is a good explanation why it is actually not needed. It is just a good practice to think about the possible next implementation, instead of modifying the current one, just create a new one from scratch and delete the old implementation.

Then you will be using the O principle, you dont need to modify the source code of classes that use the "interface", because you will be deleting the implementation, not the contract and also the L principle, because you will be using subtypes and the program will continue working.

Had to comment about it, good article... but still I would leave the interface.

I still hold that answer, I would leave the interface all days, everyday.

I also read this thread: https://www.novoda.com/blog/better-class-naming/ some time ago and I dont agree with that too, Impl is the name that IntelliJ provides for default implementations it is not a "naming convention" just lazy developers who did not want to modify the default name and create a fast implementation using the magic of IntelliJ -> "alt/option/ctrl + enter"

Just name it... I dont know, ConcreteInterface or AndroidInterface or PlatformBeingUsedInterface or leave the Impl, there is nothing wrong with that.

Also... _we should not care about the concrete class names, the contract that we expose is what we care about!!!_

Also, if you program based on the interface, that contains the "good" name (based on the article), you dont care about the implementation, we should not care about the implementation, I dont care about logic of the Presenter if I am doing View logic, you care about the contract, if it says: "presenter.doSave()"... you know that you will save data, you dont care if it will be slow, fast or if it is really going to go to a database or if it is an empty subroutine, you know that it will call the save subroutine and if it is empty that's okay, you are hitting the presenter, your layer should not care about what will the next object/layer do, only its responsibility.

Sorry for reviving this thread I saw a question over this tweet:
https://twitter.com/donnfelker/status/788842213283274752

and an answer: https://twitter.com/Guardiola31337/status/788846661791547392 with a link to this discussion! :P

PS: if you really do TDD (tbh I dont.), just create the contract and then you will just following the contract, mock the interface and everything is going to be created progressively, you will create the test case for the user of the contract, then you will create the test for the implementation of the contract and mock any dependency using just an interface, you will not care about code. Even for lazy programmers that dont want to write a lot... it is even simpler to create an interface:

even if we go and compare workflows with shortcuts it is simpler to create the interface!! haha.

this would be my workflow in macOS and IntelliJ/Android Studio when doing some TDD:

  • write any unit test and the test case, add any service/layer/object name that is going to be an interface
  • cmd+1
  • cmd+n
  • type class
  • down arrow until "interface" is selected
  • type the return type (no public access method over interfaces, it is useless to write "public")
  • type that gorgeous name of the method that you need
  • add any param (and even if you have already implementations use your ide to add params and refactor correctly instead of doing it manually)
  • cmd-[ (back in IntelliJ)
  • continue with your lovely Unit Test

Come on, compared to:

  • write any unit test and the test case, add any service/layer/object name that is going to be a concrete class
  • cmd+1
  • cmd+n
  • type class
  • type the access type
  • type the return type
  • type that gorgeous name of the method that you need
  • add any param (and even if you have already implementations use your ide to add params and refactor correctly instead of doing it manually)
  • add empty ugly {} or { return null }
  • cmd-[ (back in IntelliJ)
  • continue with your lovely Unit Test

BTW: I love this kind of conversations over internet, please invite me :(

tl;dr in english:
to be honest, I would leave the interface, I dont see the "noise" on using a contract instead of a concrete class, if you use a simple text editor it is not an excuse to say: "that's noise" because you would see (probably because everyone else that is not a super hackyhackerhackman uses Android Studio or IntelliJ while doing Java) the same contract name and the suffix Impl or if you are not lazy just ConcreteInterfaceName or AndroidInterfaceName (or I prefix if you do C#), you would only go to the interface if you are looking for the contract.

"the tl;dr is tl;dr": https://en.wikipedia.org/wiki/Dependency_inversion_principle

tl;dr in spanish:
La neta, yo dejaria la interfaz, no veo el "ruido" en tener un contrato simple en lugar de una clase concreta. No es excusa tampoco que usas un editor de texto basico y que "confunde" por que verias el nombre del archivo seguido de "Impl" (por que todo mundo que no es un super hackity hack man usa IntelliJ o Android Studio) ó cualquier nombre como AndroidInterfaceAqui ó InterfazConcretaAqui ó el prefijo "IInterface" si usas C#... solamente irias a la interface si buscas el contrato.

"el tl;dr es tl;dr": https://en.wikipedia.org/wiki/Dependency_inversion_principle

@maxcruz

This comment has been minimized.

maxcruz commented Oct 24, 2016

Hola a todos. He leído sus comentarios y antes que nada deseo agradecerles pues me han servidor muchísimo para cuestionar algunas decisiones de diseño que tomé en el pasado y evaluar mejor mi actual implementation.

Lo primero que me gustaría aclarar es que estoy completamente de acuerdo en que es un error llamar a las interfaces con el prefijo I o a su implementación con el sufijo Impl, en la mayoría de casos es una señal de sobre abstracción, usualmente estas interfaces no agregan valor y se pierde la oportunidad de nombrar el código con significado (nombrar es una de las partes más difíciles en el software, porque nombrar artefactos es diseñarlos)

Sin embargo creo que en el caso de MVP, la interfaz usualmente se utiliza con un propósito diferente al de simplemente abstraer un concepto. Pensando en las interfaces MVP como contratos entre los componentes, parte del valor que aportan a nuestro código es resaltar la relación que existe y el patrón que estamos siguiendo. Por ejemplo:

public interface LoginMvp {

    interface Model {
      ...    
    }

    interface View {
      ...    
    }

    interface Presenter {
      ...    
    }
}

Esta interfaz más que describir la abstracción de un concepto en particular (Login), está abstrayendo un sistema. Un desarrollador nuevo que se encuentra con el código al menos entenderá la intención de su predecesor de seguir el patrón MVP y le resultará simple identificar los componentes que se relacionan en esta parte del código. Por lo general tendremos muchos modelos, vistas y presentadores.

Creo que la mayoría estará de acuerdo en que uno de los problemas más grandes que tenemos los que trabajamos en Android es la falta de un playground o una arquitectura clara a seguir (uno rara vez se encuentra dos aplicaciones hechas de la misma forma). En este sentido pienso que vale la pena hacer esfuerzos que faciliten entender la arquitectura de la aplicación.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment