Android-cleanarchitecture: AutoLoadImageView breaking the rules?

[Neoklosch]

Hi guys,

at first, thanks for the great project. I enjoyed to read about this topic and fiddle around with your project.
So I think I get the main concept, but one think bothers me: the AutoLoadImageView.

It seems to me, that these view component breaks with the basic concept of the clean architecture.
Here we have model stuff like the cache and network access, as well as a lot of business logic like checking the internet connection and changing the loading logic. What's the reason for that? Thanks for your help in advance.

[Trikke]

I think there no issues here. It is just a matter of how you look at things. The caching mechanism used doesn't necessary belong in the data layer. If for instance you would have an application in which the bitmaps from this cache would be used in other parts of the application (eg: editing/cropping), then it would make sense to have some kind of BitmapRepo/BitmapCache in the data layer. I think that you should not look at it as "oh, it accesses the disk so it must be in the data layer". The "data" is not about files, bitmaps or objects, but about the entities that your app uses.
In this app "users" are the entities on which the app revolves, so anything related to storing/retrieving these entities belongs in the data layer. The bitmaps that are cached by AutoLoadImageView are not entities used by the app, so it is normal to have this in the presentation layer.

If you would use a library like Picasso or Glide, it would also be a dependency in the presentation layer, but these libraries also do a lot more then just showing an image (also caching, disk/mem access and more).

I hope i've helped here.

[Neoklosch]

Hi Trikke, at first thanks for your comment.

If it would only be the caching mechanism I would agree. But there is also the basic image loading functionality.
Using a library like Picasso or Glide is another topic.
Libraries have their own architecture and logic. Hopefully the library uses a clean architecture as well as your app. ;)

But if I implement an image loading functionality, I want a single point of responsibility.
So if I change the server/url or a method to load my data, I want to do it in one place.
With a component like the AutoLoadImageView I have to do it in several places again.
If I have more than one component with a loading functionality, the complexity will increase, as well as my effort to refactore my code.

Next problem is to test such a component.
One of the basic idea of the clean architecture is to have easy and well testable modules.
Now we have to write UI test for the components, as well as unit tests for the logic and creating mock data and so on and so on. All for a single class.
That is not what the clean architecture represents.

What I agree with is that not "every" data has to be in the data layer, but nearly all of them.
An image is an entity for me, especially if I load them from elsewhere.
Imagine you want to extend your "image entity" by a title and want to display them in the AutoLoadImageView.
Than you have to touch the whole class.
What if you want both, image with and without title. Than you have to copy your class and have redundant code.
What if I want to extend the image loading by the possibility to read you local sd card?
And so on. There are several other possiblities.

In detail I'm fiddle around with the creation of a simple image gallery which uses a clean architecture.
There I want the possiblity to load the images from the local storage as well as from a webservice.
I think it would make no sense to create components for that, even if I only load a single image.
In my opinion it's absolutely necessary to have the loading logic in the data layer and hand them over to the view/component.

[Trikke]

First off, you shouldn't look at this project as a holy grail on how to implement a clean architecture, this is just an example and meets only the specific demands of what the author intended it to have. It is by no means a finite example.

External dependencies can be designed in a lot of more ways then "clean architecture". But that doesn't matter because you are including the entire code of that library as just a dependency in the presentation layer. if you would print out the entire code of that layer, you would find data access classes in your presentation layer. But it is still valid since it is only used there to satisfy one demand (ie: load and show images). If demands change, you look to change the implementation.

So if I change the server/url or a method to load my data, I want to do it in one place.
With a component like the AutoLoadImageView I have to do it in several places again.
I do not understand, you would just use something like imageView.loadFromurl(myurl). And if you would change the url, you would just write imageView.loadFromurl(anotherurl), no need to touch that class. That class has indeed a single point of responsibility.

Your testing argument seems a bit overthought. A component like this is still testable. You can test if it loads bitmaps correctly, compare them in tests and so in.

What i think is happening here is that because you are actually trying to create an image gallery app, your demands for what "data" is, differ from this sample. And the implementation of that is entirely in you hands.
Usually, bitmaps are cached in the presentation layer because they are only used when displayed. If you would use that bitmap data for other things, then it belongs in the data layer. Remember, a bitmap is just data, the thing you call "image" as an entity is not the same as the bitmap data.
If you don't use that bitmap data anywhere else in your app, the caching of it can still stay in AutoLoadingImageView. If you need images as an entity with properties, you would create an ImageRepository and an ImageEntity which would hold the url and the title in represents. A Use Case would interact with the ImageRepository to get an ImageEntity and you'd use the ImageEntity's url property to pass it to AutoLoadingImageView so it can load and show it. If you want to show a title with it, you'd probably create a custom xml which would have both a AutoLoadingImageView and a TextView and make a custom view from it. A presenter would get the ImageEntity from the use case and then pass it to the custom view, in which you would set the correct text and let the image view load the url.

It all depends on your demands.

[Neoklosch]

Wow, thanks again for the fast and verbose response.

First off, you shouldn't look at this project as a holy grail on how to implement a clean architecture, this is just an example and meets only the specific demands of what the author intended it to have. It is by no means a finite example.

Yeahr, for sure, I know that. But my primary goal is to understand the concept in detail. For that I have to use the architecture strictly. If I get the basic idea at the end, I can start to modify the architecture and adopt it to fit my needs. But in my opinion, as long as I don't get it, I have to follow it as much as I can.

External dependencies can be designed in a lot of more ways then "clean architecture". But that doesn't matter because you are including the entire code of that library as just a dependency in the presentation layer. if you would print out the entire code of that layer, you would find data access classes in your presentation layer. But it is still valid since it is only used there to satisfy one demand (ie: load and show images). If demands change, you look to change the implementation.

Yeahr, I'm absolutely agree with you. For sure does a library have his own architecture. That's the reason why I said "Libraries have their own architecture and logic". If I want to use a library, I will implement it in the way it forced to be used. But libraries are absolutely not the topic we should discuss about. Let us focus on the Android-CleanArchitecture project.

I do not understand, you would just use something like imageView.loadFromurl(myurl). And if you would change the url, you would just write imageView.loadFromurl(anotherurl), no need to touch that class. That class has indeed a single point of responsibility.

Ok, I have to go depper into it. Let us assume we have a server which some API calls as well as a CDN. The CDN provides the images. The whole API is covered with an authentication like OAuth or something else. So everytime we call a resource, API call or CDN data (images only to simplify the example) call, we have to send out OAuth token or have to relogin. So I create my data layer with my specific network loading functionality. I also have a cache to reduce the traffic. There I cache the JSON responses, as well as my CDN data. I use the "repository pattern" to select the source which fits the best. If an entry is in the cache, I will load it from there, otherwise from the server. Pretty easy so far. Let us assume we have a component like the AutoLoadImageView. At first I have to reimplement the whole logic. Loading, caching, authentication, all of the stuff I already have. And now I want to change something. For example my company decide to change the authentication method. Now I have to modify my code in the data layer and in the AutoLoadImageView. That's redundant and have no single point of responsibility. If my component uses the methods from the data layer, than it would be a single point of responsibility in my opinion. To came back to my initial example, if the url changes, the domain for example, I have to touch several class in my project. I know it would be pretty easy to place the URL in a constant and use them in all the places, but in my last comment I decided to choose an easy example. Now it seems to be necessary to be more detailed. I hope it will help.

Your testing argument seems a bit overthought. A component like this is still testable. You can test if it loads bitmaps correctly, compare them in tests and so in.

For sure can I test the component. I can also test an "old styled" Android Activity. But again, to understand the clean architecture I have to use them as strict as I can. And as far as I understand it correctly, the idea is to have well testable and independent components. To reach that, we want a separation of concerns. UI in the UI layer, data in the data layer. So you can test each module by it's own. UI tests in the UI layer. No need for UI tests in the data layer. If I have a component like the AutoLoadImageView, than I have to mix up my test. That's also not the way of the clean architecture idea, right?

image gallery

I think with my previous example I covered exactly what I want to describe with my gallery app. Loading from an URL, as well as loading from a local sd card and caching to increase the speed and reduce traffic and waiting time. Basic example. And as you mentioned correctly I can do it in the data layer and so on.

It all depends on your demands.

Yep, I know. But again, that repo is a basic example of this architecture, where I want to learn from. In these concrete example I found a class, the AutoLoadImageView, which in my opinion breaks with all the rules of themself. That's why I'm asking or think that this is an issue. To show other people like me, how to implement thinks, the whole project should be well structured and consistent, right? This class seems not consistent to me. It doesn't matter if I can change it or adopt it. This project should be a guide to help me starting with the topic, right? If I use it in a real project and understand the concept in detail, than I can start to change it.

So I think I got all you want to say, but for this concrete example I think this class is inconsistent and worth to rethink.

But again, thanks for your response.

[Trikke]

Hi man,

i'm glad you appreciate my responses and i can only be glad that you have an interest in this and are eager to learn.
Going from your response, it seems we have to talk less about the implementations seen in this example (as they only fit the author's demands) and more about the ideas behind it. Don't try to make a copy of this example and start modifying it to fit your demands, because they will surely be vastly different to the example, as you already stated. What seems to surface for me is that you look at this example a bit to rigorously. You have an issue with the AutoLoadImageView component because you see that it caches bitmaps to disk, and think "this belongs in the data layer" because of that. But the data layer is not about "all stuff that comes from network or disk". But you have to think about data in more of an abstract way. "Data" in this example is "users". From an application perspective, bitmaps are not "data", they are just bytearrays that are only shown by the UI. There is no demand that states that these bytearrays are important to the application and can be used in other parts of the application.

As you said, your demands on image loading seem to vastly differ. In such a complex case with CDN's and more, it makes more sense to implement a CdnService to take care of all that authentication and retrieve bitmaps from the CDN. You then inject that in a Presenter and use it to load bitmaps in a normal imageView. But again, even such a service can live perfectly in the Presentation Layer. Caching is not restricted to one layer. Caching of bitmaps can be in the Presentation Layer and caching of Api Responses could be in the Data Layer. If you have a demand for which a CdnService is used both in the Data Layer (eg: to get "users") and in the Presentation Layer (eg: to get "bitmaps"), then a common approach is to have an extra layer to implement such cross-cutting concerns. See the image below for more info. Communication can be a complicated process that can be used by multiple layers.

In a gallery app, "images" would be classified as "data" because they are integral to such an application. But again, images != bitmaps. So the caching of bitmaps can be in the Presentation Layer, but the caching of ImageEntities (with titles and other properties) live in the Data Layer inside a repository.

To reach that, we want a separation of concerns. UI in the UI layer, data in the data layer. So you can test each module by it's own. UI tests in the UI layer. No need for UI tests in the data layer. If I have a component like the AutoLoadImageView, than I have to mix up my test. That's also not the way of the clean architecture idea, right?

The thing is that you define "data" as actual bits and bytes and the stuff that is read to and from disk or memroy. "Data" is a concept, it is about the Entities (think real world stuff, like users, groups, types of salad) your application will use, not about the actual data and how it is implemented. Back to the case of the AutoLoadImageView, since bitmaps are not actual "data" and are heavily tied to a view, for all its intents and purposes, it can live just fine in the Presentation Layer. Separation of Concerns is not about "UI in UI and Data in Data", but more about one class have one purpose. The purpose of AutoLoadImageView is to fire of a request to fetch an image and show it. It uses an ImageDownloader class, which only has the purpose to get the image from network and a DiskCache which does nothing else that to get bitmaps from disk. They are 3 separate classes handling their own concerns. If you want to use a memory cache, you'd just replace the DiskCache class with a MemoryCache class and implement it. This is Separation of Concerns on a small implementation detail (ie: load external images in a view). It would be bad if AutoLoadImageView didn't have those separate classes and tried to do all that work by itself in one class.

I hoped this helped you a bit. A good read on general practises in mobile application architecture is this free ebook. You can ignore the Microsoft stuff, but it gives a good idea on structuring and common pitfalls.

[android10]

@Trikke @Neoklosch great discussion: nice feedback and input.

In conclusion, the AutoLoadImageView is a view which encapsulates some logic of course, but in the end it is an implementation detail. It is not solving any domain problem.