y = f (x)
As we know from our calculus courses that vertical line test could be used to determine if there exists a function. It ensures that for a single domain value (x), there exists a unique range value (y)i.e. classic function is Single Valued. The function should always evaluate the same value no matter how many times we use the function i.e the function is Stateless. It is perfectly possible for two domain variables (x) to have the same range value (y) e.g. Identity function.
In modern programming languages, the concept of function has been adopted in a modified way. This pushes us to discuss why we have used "modified way" here. In order to make life easier for developers some of the classic function restrictions were relaxed by classic programming languages when they introduced concepts like Global Variables and multiple parameters. Object Oriented programming languages attempted to limit a few things but they started calling it method. The method has accessibility to all the instance and type members of the same type. It also has visibility of most members of the parent type in an inheritence relationship.
There is certainly no doubt that the life of a regular programmer developing simple application is a lot easier now. But advancement in technology which provided us the flexibility to have multiple flows of execution even within the same process. They multiple flows of execution were named as Threads and the concept was termed as Multi-threading. Now the method can be CALLED by multiple threads at the same time. It is possible that two or more threads are executing the same method simultaneously. Since we are using the instance members directly in the method's code, any change in the member would be reflected for the other which might cause unexpected result for the other threads. The problem happens just because that our methods are not free from side effects. They have a defined purpose to evaluate some result and as a side effect they can use / modify instance members. This is a given feature of Object Oriented Programming technology. Modern programming languages provide various synchronization techniques (such as locks) to avoid ending up in an unexpected state.
Side effects are lies. Your function promises to do one thing, but it also does hidden things. Sometimes it will make unexpected changes to the variables of its own class. Sometimes it will make them to the parameters passed into the function or to system global. In either case they are devious and damaging mistrusts that often result in strange temporal couplings and order dependencies. [Robert C. Martin - Clean Code: Chapter 4 - Functions]
The synchronization techniques really help us avoiding the side effect related issues of multi-threading. But the issue is that they help us fight with the symptoms and not the actual cause. The cause is unnecessarily relaxing the limitations of functions for the ease of development. There are several things to consider but we can start with avoiding use of any instance level member as much as possible in our methods. Doing that would definitely help us in avoiding the side effects. We will be making less use of synchronization techniques. Not only the performance of our code is better but the code is more naturally organic and readable. It is also quicker to program.
Oliver Sturm has a number of suggestions regarding defining pure functions. We are discussing one of the suggestions in this post. i.e.
Define thou's private methods as static.
Basically using the static modifier with a method is a reminder to the compiler to check whether any instance member is accessed in the method. If it is so, then it issues a compiler error stating the problem. This can serve as a warning to the developer making any change to the code later on. All the future refactoring attempts would also be considering the static nature of the method in mind.
The following is a type called CoffeeCard. This card can be used to pay for the coffee so that you don't need to carry cash to the coffee shop as long as you are maintaining enough balance. The type allows us to manage card balance. It allows us to add balance and charge for the coffee for a certain balance. We can check balance anytime by calling GetBalance() on the type. AddBalance() and ChargeCard() utilizes private methods to update balance and push the changes o the database.
As suggested above, let's update the private methods to static.
As soon as we change the method to static, the JIT compiler starts complaining about the use of _balance field, which is an instance member. As we know that any instance member from the same type cannot be used directly in a static method of the same type.
Basically the solution of this problem is easier than we think. We just need to change the signatures of private methods to accept the balance as a parameter. Since we need to update the _balance field after the operation, we can return the calculation result which can be used by the caller of these methods to update the instance member [_balance field]. Since these are our private methods, we don't even need to worry about outside world while refactoring them as it's none of their business how we implement functionality within our type. As long as we are not changing the behaviors of instance, we should be good which, in this case, we are not.
Let's see how we can use these methods in the public methods of CoffeeCard type.
Now our private methods are thread safe, they are deterministic with no side effects, they are context free. The biggest of all achievements, they are thread safe.
Does the method really belong to this type?
Resharper's suggestion is mainly because of the idea, "Since this method is not using any instance member, it is not an instance member itself and we should rather be specifying it as a type's member". In C#, we assign type membership using static. If it were VB, we would have specified the methods as Shared.
After you have updated the method to be static, just ask this question to yourself: Does this method really belong to this type? If the answer is Yes then you are done here. Smile and move ahead. But if the answer is No then we should continue on the road of design improvement. It seems that you were keeping this functionality here. It happens when Single Responsibility Principle is avoided. If this is some code which goes above and beyond the type's responsibility then we can move this to some other type. We might introduce new types and move this functionality there. Doing this would also improve our unit testing as we will be adding this functionality in public methods of the other type and hence we can unit test that.
If we want to add a new type (class), then we need to decide where we need to add this. We keep the visibility of the type to as the minimum. This depends on the the other types which might need the functionality of this new type. The choice of access modifier of a type is an important decision. Keeping all the types as public is exposing yourself to the infinite number of ways the types can be used. If we don't see that any client would be using our type from outside the assembly, we should never be making it public. If the new type is not expected to be used outside this scope of the class it is refactored from then it would make more sense to be keeping this class nested in the original class as follows:
Here we have kept the BalanceCalculator's access domain as private. This is allowed for a nested class in C#. Since we don't think that the class can be used by any code outside the parent class so it seemed like the best decision. The refactoring also made us realize that adding the balance and charging cards is not just an arithmetic operation. We might also need to consider the effect of any on-going promotion and discount.
Resharper Support
Resharper also provides support to suggest about the members which can be declared static. For our case, the suggestion can appear as follows:
This is definitely a configuration option and we can change the severity of resharper option to a value other than suggestion as well.
FxCop Rule CA1822
FxCop Rule CA1822 also suggests marking those methods as static which have no reference to any instance member.
http://msdn.microsoft.com/en-us/library/ms245046.aspx
No comments:
Post a Comment