Influencing AOP with attributes via IoC; code-smell or elegant?_问答_开发者

I'm using StructureMap at the moment, generally with convention-based (Scan()) auto-configuration, and I'm looking to add decorator-based caching into the pipeline.

If I configure it manually that is fine, but Scan() is just so convenient when you get lots of dependencies... I'm toying with noting cache suggestions on the interface(s), for example:

public interface IFoo {
    [CacheDuration(20)] // cache for 20 minutes
    string[] DoSomethingReusable();

    SomeType DoSomethingNonReusable(int key); // not cached
}

with the idea being that by adding a custom "convention" to StructureMap's scan开发者_StackOverflow中文版ning (pretty easy) it can spot that one-or-more methods are decorated for caching, and automatically inject a generated caching decorator into that type's pipeline (generating a cache key from the interface/method name and parameter values).

On the plus side it makes adding caching very painless - just decorate the interface a little; but is is a code smell? And/or am I duplicating something that is already solved?

Attributes are OK if you're aware of two major factors:

inherently decentralized structure, instead of in one place you have your metadata sprinkled on each and every component it touches
rigid compiler constant structure. If you suddenly decide that your cache duration should be 10, not 20, you have to recompile all your code. You could route attributes to things like config, but that's working around the attributes, and at this point you should really reconsider whether using them was the best idea in the first place.

As long as you're aware of these issues, and OK with them, than go ahead.

As far as I understand the question, you are mainly considering adding the Attributes to make container registration easier. You are still implementing the caches using the Decorator design pattern, which is, IMO, the correct way to implement caching.

We do the same in Safewhere, but we use convention-based registration instead. We also use Castle Windsor, so I don't know if this is possible with StructureMap, but we simply scan the appropriate assemblies after anything named Caching*Repository and register those as Decorators for the real Repositories. We also have a convention-based unit test that verifies that all required Caching Repositories are present.

Whether adding the custom attribute is a code smell or not depends on the degree of reusability you require of your code. My rule of thumb is that I want to be able to wire everything up with Poor Man's DI. I still use a DI Container instead, but this rule provides me with a sanity check.

In general I don't like coupling my code to a particular container, but I can't really figure out whether you are doing that here. It depends on whether or not you need to reference StructureMap to define the custom attribute. If you must reference Structure Map, I would consider it a smell.

The only drawback I see here is that there is no central spot where objects are cached. Usually there is one specific layer where caching is done. With your technique, this is not the case.

I also think it's the responsibility of your code to decide whether something should be cached or not, not that of a class/interface.

But to answer your question, is this a code smell? I'd say no, but it might be confusing for new developers to get the hang of this system once it's used in a lot of places.

I can't say I totally understand everything, but for my 2 cents, it seems arguably okay; but I just wonder whether or not it's easy to change it without recompilation. And maybe you would wish to change that cache duration via config entry, and not a compile. I'd probably store that data somewhere where I can configure easier. (Perhaps I've misunderstood though ...)

I tend to agree with what was said about the caching having to be decided by your code but another scenario that I see that you might want to take into consideration is the "What needs to be cached, the entire object?

You might want to toy with the idea of having properties to set which members you don't want to cache of an object.

Maybe an Attribute

[Cache(Duration=20, Location(...))]

I think something to consider whenever you are using attributes is the coupling of the intent with the class. The point being that you cannot use that entity in once place with a cache duration of 10 seconds and another place with a cache duration of 60 seconds. IMHO this is always the trade-off with attributes of any kind.

I'm a bit late to the party on this one but I've been thinking about this problem recently and in general, using attributes for caching is a code smell.

Here's a couple of reasons why:

Caching by attribute can get you into a bit of a "tail-wagging-the-dog" scenario as it has the potential to change the design of your code as you implement methods that lend themselves to be used by the caching attribute, particularly when you get into complex scenarios.
Caching by attribute is dangerous because, whilst the process of adding items to the cache is generally the same (i.e. cache.Add(key, value)), the process of determining the key and the item to be inserted is not always the same and usually requires some sort of logic which isn't always generic. For example, it's often thought of to generate the cache key by using the parameter values, but consider the scenario where a DateTime parameter is being used and the caller passes in DateTime.Now... the return value might always be the same, but the key generated by the parameters will be different and will generate a new cache object for each call. You could then change the design of the method but then you're getting exactly into the problem described in 1
Generally speaking, caching is used to optimize a specific scenario and it is rare that the reason an item is entered into cache is the same. Suppose you have a catalogue of items that rarely changes, you might decide to load the entire catalogue into cache when the application starts and cache it for 12 hours, but you might decide to cache the user details of a user only after they log in for the first time. The logic required to retrieve these items and then add them to a cache is completely different and doesn't lend itself to being captured in an attribute.
If you had a cache attribute, you would then also need a way to expire cache items when they change. Assuming that you have a cache attribute, you would most likely have a "CacheExpiry" attribute which would have the same problems above, but you'd also have to then come up with a common way of generating cache keys between methods because it'd be highly unlikely that your parameters for updating or deleting an object would be the same as those for adding an object to the cache.

Bottom line is that on the surface Caching by attribute seems like a good idea, but in practice the nature of the problems that caching solves means that it doesn't lend itself nicely to being driven by attributes.