Reputation: 37
I don't like the look of this function. Is there a way to make it look less ugly without "magic strings".
private static bool Inconsistent(AdStats adStat) {
return
adStat.Daily.Impressions != adStat.Hourly.Sum(h => h.Value.Impressions) ||
adStat.Daily.Clicks != adStat.Hourly.Sum(h => h.Value.Clicks) ||
adStat.Daily.Spent != adStat.Hourly.Sum(h => h.Value.Spent) ||
adStat.Daily.SocialImpressions != adStat.Hourly.Sum(h => h.Value.SocialImpressions) ||
adStat.Daily.SocialClicks != adStat.Hourly.Sum(h => h.Value.SocialClicks) ||
adStat.Daily.SocialSpent != adStat.Hourly.Sum(h => h.Value.SocialSpent) ||
adStat.Daily.UniqueImpressions != adStat.Hourly.Sum(h => h.Value.UniqueImpressions) ||
adStat.Daily.UniqueClicks != adStat.Hourly.Sum(h => h.Value.UniqueClicks) ||
adStat.Daily.SocialUniqueImpressions != adStat.Hourly.Sum(h => h.Value.SocialUniqueImpressions) ||
adStat.Daily.SocialUniqueClicks != adStat.Hourly.Sum(h => h.Value.SocialUniqueClicks);
}
Upvotes: 2
Views: 151
Reputation: 113402
I think by "optimization", you mean "redundancy reduction" aka Don't Repeat Yourself?
Essentially, you have a bunch of metrics. You want to check if, for any of those metrics, the value of that metric for an ad on a daily basis deviates from the sum of that metric on an hourly basis.
Once you think of it that way, you can do:
Func<Stat, int>[] metricGetters =
{
stat => stat.Impressions,
stat => stat.Clicks,
// .. etc. etc.
};
return metricGetters.Any(getter => getter(adStat.Daily)
!= adStat.Hourly.Sum(h => getter(h.Value)));
Upvotes: 5
Reputation: 8540
you could extract adStat.Hourly.Select(h => h.Value) to variable, it would reduce code amount:
private static bool Inconsistent(AdStats adStat)
{
var hourlyValue = adStat.Hourly.Select(x => x.Value);
return
adStat.Daily.Impressions != hourlyValue.Sum(h => h.Impressions) ||
adStat.Daily.Clicks != hourlyValue.Sum(h => h.Clicks) ||
adStat.Daily.Spent != hourlyValue.Sum(h => h.Spent) ||
adStat.Daily.SocialImpressions != hourlyValue.Sum(h => h.SocialImpressions) ||
adStat.Daily.SocialClicks != hourlyValue.Sum(h => h.SocialClicks) ||
adStat.Daily.SocialSpent != hourlyValue.Sum(h => h.SocialSpent) ||
adStat.Daily.UniqueImpressions != hourlyValue.Sum(h => h.UniqueImpressions) ||
adStat.Daily.UniqueClicks != hourlyValue.Sum(h => h.UniqueClicks) ||
adStat.Daily.SocialUniqueImpressions != hourlyValue.Sum(h => h.SocialUniqueImpressions) ||
adStat.Daily.SocialUniqueClicks != hourlyValue.Sum(h => h.SocialUniqueClicks);
}
I also like Ani idea, it allows even inject additional conditions from outside.
Upvotes: 1