If you intend the first badge number lookup as a cache, then you should perform a lookup with the database even if the cache didn't hit. These are the responsibilities under situation 1: Typically, in order to be consistent with a (live) database, you better let the database handle it. It also provides a different error message than if the database query didn't return a result. It can be seen as some database query caching or optimization hack. If they are all consistent, it doesn't alter the logic of the function. badge number and domain name matches what is in the database.What it means is, the following three conditions are either all-true, or all-false. Will this new precondition change the logic and flow of the whole function significantly? The answer to this question depends significantly on the relationship between the badge number data and the string lookup by domain name. In other words, what the function does is GetUserIdIfBadgeNotNull.
Single responsibility principle functions code#
If GetBadgeNumber returns null, the rest of the existing code wouldn't run. The commented code, if ever uncommented (executed), introduces a precondition for the rest of the (existing) code. I will approach this from a logic perspective. You would then use the code like this: public static int GetUserID(string domainName) Interpreting any magic numbers that come back from the service (e.g.Cache service call results to avoid retrieving the same badge number repeatedly.Calling a service to get the badge number.Comparing two users to see if they match.Parsing the domain string to get the user name.Parsing the domain string to get the domain.
Throw new InvalidOperationException("Failed to get user.") Where(u => !u.deleted & this.IsMatch(u)) & string.Compare(candidate.badgeNumber, this.GetBadgeNumberWithCache(), StringComparison.OrdinalIgnoreCase) = 0 & string.Compare(candidate.samAccountName, this.AccountName, StringComparison.OrdinalIgnoreCase) = 0
Return string.Compare(candidate.domainName, this.Domain, StringComparison.OrdinalIgnoreCase) = 0 Return _domainNameString.Split('\\').Trim() Throw ArgumentException(String.Format("No badge number found for domain ", _domainNameString), "domainName") Private readonly string _domainNameString In this case it's a class about a user, so I'll call it MyUserClass (you should probably change the name). The functions that I don't want exposed I will scope as private. That way I can have small, narrowly-defined functions that do one specific thing (have a "single responsibility," so to speak). Personally I would consider splitting the function into separate functions under the same class. I will interpret your question to mean "How can I improve the structure of this code for maintainability?" without reference to any specific principle. SRP applies to classes (or modules), not functions, so this is sort of an odd question. Any reason you're not throwing exceptions? Do your integrity checks separately.Īnd Euphoric has a point about returning 0. Only make badgeNumber part of getting a userId if you really need it to get down to one userId. I mean, what happens if (domainName) decides it wants to be sure the domainName produces a user with a userId? That will just run you in circles. That would be a pointless extra responsibility. As if you were only doing it as some crazy kind of integrity check. Where this might be a single responsibility problem is if eliminating users based on badgeNumber isn't needed to weed down to one result. If I was trying to debug this you wouldn't want me knowing where you live. There are 3 other ways to get a null user and end up here, yet this log message always blames the badge number. My biggest problem with this code is right here: message = "Failed to get user by badge number: " + domainName It should only ever have one reason to change. Having a single responsibility doesn't mean the task is simple. Unfortunately in this system that is, apparently, not a simple task. This functions single responsibility is to get the user id associated with a domain name. Message = "Failed to get user by badge number: " + domainName StringComparison.OrdinalIgnoreCase) = 0) String samAccountName = domainName.Split('\\').Trim() String domain = domainName.Split('\\').Trim() Using (var db = new SurfaceTreatment.SurfaceTreatmentEntities()) function should stay as single responsibility Do not get badge number from this function, Look at the commented codes below, is it true about the single responsibility of a function/method?ĭo this function violate single responsibility principle if those codes are not commented? public static int GetUserId(string domainName) The function below is suppose to return user ID store in database by using the SAM account name pass into it. I would like to find out if I understand single responsibility principle correctly.
0 kommentar(er)
