• 1

• 5.2k
• 1
• 30
• 47

svick's observations are spot on, however I suggest you go a bit further in your refactoring. #Suggested Refactorings# ##Naming##

• GetSomeValue is not expressive. I changed it to GetBonusDetails, which makes more sense to me, but then again I don't know the rest of your code base - just choose a name that clearly says what the method does.
• con is borderline acceptable... for clarity I renamed it to dataContext.
• the name a doesn't tell us anything about what this variable contains. I renamed it to bonus.
• the name isActive is a lie. This int is pretending to be a bool, probably because it should indeed be boolean. I don't know for sure, since the range of possible values is not apparent in your code.
  • If it only gets assigned the values 0 and 1, change the data type to bool.
  • If it can have other values, rename it to activityStatus or something similar.

(from now on I will use the corrected names in my explanations and code.)

##Exception Handling##

• I changed SingleOrDefault() to Single() to avoid a possible NullReferenceException.
• I removed your try-catch-statement: swallowing an error can turn into a debugging headache in the future. I believe that in this case you should probably create a custom BonusNotFoundException. You would then catch this exception at a higher level and notify the user.

##Output Parameters## I agree that it would make more sense to initialize the output parameters with null since you made the effort of making them Nullable. But if you take a step back, you realize how ugly the use of out parameters is in this situation. So I deleted them and made GetBonusDetails return an instance of a new class designed to hold two pieces of information: maxGiving and isActive. I called it BonusDetails, but I'm sure you can find a much better name for it.

##LINQ Query

I rewrote your query as a query statement because I find the syntax less convoluted. However, using a method chain is also all right if it is more readable for you.

#End Result#

##GetBonusDetails method public BonusDetails GetBonusDetails(int bonusID) { var dataContext = new BonusModelDataContext(); var bonus = from b in dataContext.Bonus where b.ID == bonusID select new BonusDetails(b.maxGiving, b.isActive); return bonus.Single(); }

##BonusDetails class public class BonusDetails { public int MaxGiving { get; private set; } public bool IsActive { get; private set; }

 public BonusDetails(int maxGiving, bool isActive)
 {
 MaxGiving = maxGiving;
 IsActive = isActive;
 }
}