Title probably doesn't describe my problem too well, I'll be glad if somebody could edit it to something more appropriate. Anyways:
I got a component that is supposed to return product price given its id
. It implements an interface like this one:
interface IProductPriceFetcher
{
double GetPrice(int id);
}
Now, the price can be fetched from 3 different sources:
- web service
- directly from website source code (scrapping)
- as a final fallback (both webservice and website are inaccessible), most recent price from local database is returned
To play around this 3-different-sources issue I've implemented class like this:
class MainFetcher : IProductPriceFetcher
{
public double GetPrice(int id)
{
var priceFetcher = this.factory.GetWebServiceFetcher()
?? this.factory.GetWebsiteFetcher()
?? this.factory.GetLocalDatabaseFetcher();
return priceFetcher.GetPrice(id);
}
}
Each method from factory returns IProductPriceFetcher
of course, with side note that first two can fail and return null
; I assumed GetLocalDatabaseFetcher
will always return meaningful object tho.
My "general wondering... ment"
Upon successful webservice/website call, I want fetched price to be inserted into local datbase, as a future fallback case. Now my question is: which part of code above should be responsible for that? Should it be one of the concrete web fetchers that returns price? Or "aggregator" fetcher (MainFetcher
), as it also has knowledge over what's the source of price? Should I raise some event? Inject yet another interface with DB calls? Change design to better?
Why did it even occur as an issue to me? Well, I tried to keep the code clean (worry not, it's only pet project for开发者_C百科 my spare time - exactly for solving problems like this) possibly with SRP/SoC in mind. Now I seem to have problems switching from this mindset - I mean, how could possibly something that fetches webpages also be doing database inserts? Oh, come on! :)
If you want to have a super-decoupled design, I would implement a Decorator class like the following and use it to wrap both the WebServiceFetcher and the WebsiteFetcher:
class DatabaseCachingFetcherDecorator : IProductPriceFetcher
{
private readonly IProductPriceFetcher innerFetcher;
public DatabaseCachingFetcherDecorator(IProductPriceFetcher fetcher)
{
this.innerFetcher = fetcher;
}
public double GetPrice(int id)
{
double price = this.innerFetcher.GetPrice(id);
if (price != 0) // or some other value representing "price not found"
{
SavePriceToDatabase(id, price);
}
return price;
}
private SavePriceToDatabase(int id, double price)
{
// TODO: Implement...
}
}
Then your factory would implement the following methods:
public IProductPriceFetcher GetWebServiceFetcher()
{
return new DatabaseCachingFetcherDecorator(new WebServiceFetcher());
}
public IProductPriceFetcher GetWebsiteFetcher()
{
return new DatabaseCachingFetcherDecorator(new WebsiteFetcher());
}
This design decouples your actual fetchers from your caching mechanism.
EDIT: I misread your design slightly with this answer, as I assumed that the GetPrice method would return some sort of NULL value if the price couldn't be fetched, rather than the factory returning a NULL value. I think the factory returning NULL smells a bit, since a factory's responsibility is to reliably return objects. I would consider changing your GetPrice
method interface to return a double?
perhaps, to allow for "price not found".
It sounds to me like if you needed a "Cache". Caching is typically implemented either as a kind of aspect or dependency which you inject into your Fetcher
implementation. Below I assume IPriceCache
with sort of IDictionary
interface, but you could of course insert whatever abstraction you require. I also suggest to abstract away the data sources for price fetchers...:
class MainFetcher : IPriceFetcher {
IEnumerable< IPriceSource > mSource;
IPriceCache mCache;
public MainFetcher( IEnumerable< IPriceSource > pSource, IPriceCache pCache )
{
mSource = pSource;
mCache = pCache;
}
public double GetPrice(int pID)
{
double tPrice;
// get from cache
if (mCache.TryGet(pID, out tPrice) {
return tPrice;
} else {
// throws if no source found
tPrice = mSource
.First(tArg => tArg != null)
.GetPrice(pID);
// add to cache
mCache.Add(pID, tPrice);
}
}
}
精彩评论