views:

37

answers:

2

We have two domain types: Users and Locations.

We have a method on the LocationRepository: GetUserLocations().

The existing implementation:

var user = UserRepository.GetUser(userId);
var locations = LocationRepository.GetUserLocations(userId);

To me it makes more sense to retrieve the locations associated with a user from the User type i.e.:

var user = UserRepository.GetUser(userId);
var locations = user.GetLocations();

I think the latter implementation reads more cleanly and as an API client I have to deal with fewer types (i.e. the LocationRepository is not required). On the other hand, there will be more code to maintain as I have to write the "facade" to the LocationRepository.

Should I act on my instinct and create a facade on the User type to the LocationRepository, or should I be happy with the status quo and live with a sequence diagram that "feels" wrong to me (i.e. retrieval of the location information feels like it is being retrieved from the wrong "point of view")?

+1  A: 

I would approach this from a perspective of maintainability. I agree that doing it with the facade on LocationRepository would "feel right", and would probably make the code slightly more readable.

The tradeoff, as you said, would be more code to maintain. But how much code are we talking about? Is it a lot, and would you have to update it often? Or can you write it once and forget it, and have it be easily unit testable? If it's the former, just swallow it, go with the current implementation, and remind yourself it doesn't really affect functionality. If it's the latter, it might be worth putting in the effort for that good feeling and more readable code elsewhere.

Neville Flynn
+1  A: 

It is surely possible to model our stuff something like this:

Universe.Instance.Galaxies["Milky Way"].SolarSystems["Sol"]
        .Planets["Earth"].Inhabitants.OfType<Human>().WorkingFor["Initech, USA"]
        .OfType<User>().CreateNew("John Doe");

Maybe the repository instances should not be visible to the "end developer" and be encapsulated in the model.

However, because we might not have easy access to the Universe.Instance, we do need one or more "entry points" to where to actually get any data from.

UPDATE:

I think on the one hand it should be a goal to keep the number of there "repository facade entry points" as low as possible because that comes closer to the real world, as there is supposed to be only one "Bing Bang" where everything comes from and which spawned all existing data ultimately ;-) ... That said, on the other hand of course today's systems are always big compromises which we have to make, because the ability to model the real world is limited, there are performance implications and so on...

One way you can go in your concrete example however is to use your repositories for always retrieving fresh data, as in:

LocationRepository.Instance.GetUserLocations(userId);

...whereas you use your User model class to hold the result in a property, as in:

var locations = myUser.Locations;

This property would use lazy load technique to load the data from the LocationRepository at first demand and then hold the result. This communicates that the locations are only loaded once, which makes things for developers who use your library easier. You can then decide if you want to make the LocationRepository.GetUserLocation(userId) also visible to the end developer or not. Keep in mind that when going that route you will also need to build some kind of implicit as well as explicit refresh mechanism and lifetime management.

This over-all approach has proven to be very useful for me. However, the async world of Silverlight et ál now adds some new caveats, as such properties cannot be refreshed instantly and synchronously with a new value in one line of code anymore. When we request a refresh, we now have to leverage binding techniques and/or use a callback to then be able to further process the refreshed value.

All in all, the ultimate goal I believe still would be to see for example a UserRepository just as another regular domain type, apparently with the responsibility to create new single User instances and add them to the storage of users, as well as to provide filtered views (queries) on all available users. It would be acceptable that myUser.Locations as well as myLocations.ByUser["John Doe"] hold a reference to the same result. This UserRepository could then merely be a property of another class responsible for holding it, like CompanyStaff for example. Carrying that idea further is what brought me to that Universe.Instance thing. ;-)

herzmeister der welten
Your response is a little ambiguous. Are you favouring the creation of a facade (which in itself would be an "entry point"), or are you saying that any system is one big compromise and so I may as well use the location repository directly?
Ben Aston
A little bit of both. ;-) ... I updated my answer with a richer explanation.
herzmeister der welten