Showing posts with label Object Oriented Design. Show all posts
Showing posts with label Object Oriented Design. Show all posts

Tuesday, January 17, 2012

Object Versioning is an Open Design Problem


This unsolvable maze is a local food from Bangladesh, known as Jilapi
Photo credits to udvranto pothik
Object Versioning is often required by a business rule, for example, to maintain an audit trail or to be able to revert to a previous version, etc. This is the 3rd time in my career where this Object Versioning requirement made me think like -
There's gotta be an easier solution! 
But, I am yet to find one. So, I am thinking it's one of those open design problems, may be.

To clarify the requirement with an example, let's consider the following scenario:

A lawyer is preparing a document for one of her clients using a software. On January 17th, she needs to take a look at the version of the same document from May last year so that she can backtrace some changes that took place during these months.

Lets assume the lawyer is using a software that stores the documents in a relational database with the following schema.
A Document has many Evidences, each provided by an EvidenceProvider

Document (id, client_id, case_id, name)
Evidence (id, document_id, evidence_provider_id, details)
EvidenceProvider(id, name)
Now, given the versioning requirement how would you design your data model?

Here's a couple of points that your design should address at a minimum:
  • Going back to a version means a complete snapshot of the old version of the document. So, the version of May 1st should only bring the evidences that were there on that very day.
  • As a new version is created, it should inherit all previous evidences.
As I have mentioned earlier, I am yet to find a good data model that can take care of these concerns without over-complicating everything. Let me know if you got a beautiful solution to this problem.

However, in my latest project, the requirement is even harder. It's somewhat like this:

The lawyer may have some documents in the "work in progress version". This means, if she needs to print a document for the court, she only wants to print the last "good version", skipping the "work in progress version".

Also, when there is such a "work in progress version", she needs to attach any new Evidence to both the last "good version" as well as to the "work in progress version".

Well, now you see the design of a data model for Object Versioning becomes really messy and unintuitive.
So, here's my question to you - how would you design a solution for this?

Tuesday, November 01, 2011

Using instanceof is mostly a Code Smell

When using static programming languages such as Java, often time I have seen people writing methods that accept Object as a parameter. These methods typically don't really work on any Object, but works with multiple types of classes that don't have any common base class. Here's an example of such a method:

As you can see in this example, the process method actually expects one of CleanFloor or LaunchRocket instances. However, since the two don't have a common subclass, it falls back to an Object type. And that results in the smelly code as you see in the example.

An ideal solution would be to change the design of the classes so that you can either use a common base class or a generic method. But if you can't just do that

Get rid of the instanceof anyway!

However, this doesn't need to be smelly. Turning to basics of OO programming concepts, you will recall that there's this thing called method overloading that is specifically there to solve this very problem.

It seems so obvious in this example that you might question, why someone would use the former code? Well, I have seen quite a few of them and if you search for instanceof in your Java project, I won't be surprised if you see a few code fragments that match the former example.

Tuesday, September 20, 2011

Excess of Private Methods is a Code Smell

Private methods, when used meaningfully, are a great tool for writing beautiful object oriented code. But as many other things in life, excess of private methods is bad, too!

In my opinion, we use private methods to:

1.  isolate a block of code to be reused inside the class.
2.  extract code from another method for code readability.

Now, taking these two use cases in mind, here's an easy conclusion:
The lower the ratio of public to private methods, the harder it is to write unit tests since the "units" are potentially larger.
I don't know if there is any rule of thumb, but you will smell it when you see your unit tests require a lot of setup and assertions. Here's a code example from the Play! framework, an MVC franework for Java developers.

ActionInvoker.java

You will see there are public methods with 100+ lines. I hope you agree with me:
"The ActionInvoker.java code is not readable"
For the sake of readability, introducing private methods with good names would help. However, that doesn't eliminate any of the possible code paths from the public methods. So, if you are lucky, you will see really long unit tests with complex setup conditions and mock expectations. Otherwise, there will be no tests at all! Without any tests for such long and complex methods, use them at your own risk. I won't :(

Disclaimer: I like the play! framework a lot. However, if you take a look at their code and if you think unit testing is important, you'll see they have a lot of rooms for improvement with simple extract method refactoring.

Monday, July 19, 2010

Know Your Enemies Before They Kill You!

From darekb
You know what, this is a political world. So, enemies will seemingly look like your friends until the moment when.. well, its time for the kill! My dear readers, its time to know the enemies!

I have met with a few enemies off late. I will go one by one here.

If you are a software developer like me, you will often see this enemy, camouflaged variables, methods, classes and namespaces. For example, I have recently seen a Stack camouflaged as Visitor! I really mean this. I found a class called Visitor, so I was expecting a Visitor Design Pattern implementation or something similar, but what I got was a Stack under the hood with two methods Push and Pop! This enemies are very bad for your health, as they will keep you guessing all the time... you never know what they do from looking at their names!

Another enemy you will often see are the very skinny ones, to skinny to have any meat in them. I met some enemies like this as well. What happens when you do over engineering with interface explosion and a lot of one method classes? Is it really that difficult? Is it really a class? Is it really a package? I don't think so! You can spoil a piece of code by introducing a class/package for a single method. This enemy often surfaces because of the fact that, the design pattern book only shows classes with one/two key methods in them... which is of course not intended. But this is life... you gotta balance between class explosion and God classes... really, or this enemy will kill you someday.

I have just touched two common enemies... but there is another enemy we all are aware of, CMD+c (OS X) or Ctrl + C (Win). Its such a pain to copy a code fragment and use it in a different class... this is exactly the form of reuse that kills everything. Make sure you don't let this germ to grow, or it will outgrow you and leave you crying.

Have you reviewed your code by someone else today? If yes, keep up the good work. If not, beware of the enemies before they get you. Best of luck!

Wednesday, May 13, 2009

Unit/Functional Test Rails ActionController filters following DRY

At ScrumPad most of our controllers are bounded by filters for authentication/authorization. Some filters apply to all actions in a controller while others apply to only a few or leave out only a few. However, since we are following TDD, we need to test the filter is invoked before each of the desired action. This makes the test code MOIST (not DRY)!

Example of Moist Code:

The following example only captures two test methods. However, if you have 30 controllers like ours and on an average 5 filters at each, you will possibly find many such duplicates making your test code so moist that it becomes shabby!

class SomeController
before_filter :authenticate
before_filter :restrict_invalid_role
end
class SomeControllerTest
def test_index_redirects_without_login
get :index
assert_redirected_to :controller=>:login, :action=>:login
end
def test_index_redirects_without_valid_role
login_as(:invalid_role)
get :index
assert_redirected_to :controller=>:exception, :action=>:not_allowed
end
end

Example of DRY Code:
I came up with the following implementation to help us with unit testing the before filters. The assumption is, if your filter is invoked, it will work fine. And we are testing the filter only once. The following code is written at the end of the test/test_helper.rb.
class ActionController::TestCase
##example: should_apply_before_filter_to_actions(:authenticate, [:index, :new])
def should_apply_before_filter_to_actions(before_filter_name, actions)
if(actions.nil? or actions.empty?)
assert false
end
filter = find_maching_before_filter(before_filter_name)
actions.each do |action|
assert_before_filter_applied(filter, action)
end
end
##example: should_apply_before_filter_to_action(:authenticate, :index)
def should_apply_before_filter_to_action(before_filter_name, action)
filter = find_maching_before_filter(before_filter_name)
assert_before_filter_applied(filter, action)
end

##example: should_not_apply_before_filter_to_actions(:authenticate, [:index, :new])
def should_not_apply_before_filter_to_actions(before_filter_name, actions)
if(actions.nil? or actions.empty?)
assert false
end
filter = find_maching_before_filter(before_filter_name)
actions.each do |action|
assert_before_filter_not_applied(filter, action)
end
end

##example: should_not_apply_before_filter_to_action(:authenticate, :index)
def should_not_apply_before_filter_to_action(before_filter_name, action)
filter = find_maching_before_filter(before_filter_name)
assert_before_filter_not_applied(filter, action)
end

##example: should_apply_before_filters([:authenticate, :session_expiry])
def should_apply_before_filters(before_filter_names)
if(before_filter_names.nil? or before_filter_names.empty?)
assert false, "No Before Filter is Passed"
end
before_filter_names.each {|filter| should_apply_before_filter(filter)}
end

##example: should_apply_before_filter(:authenticate)
def should_apply_before_filter(before_filter_name)
filter = find_maching_before_filter(before_filter_name)
if(filter.nil?)
assert false, "no matching filter found for #{before_filter_name}"
end
assert filter.options.empty?, "the filter #{before_filter_name} has only/except options and does not apply to all actions"
end

private
#finds the matching BeforeFilter object
def find_maching_before_filter(before_filter_name)
filters = @controller.class.filter_chain()
if !filters.nil?
filters.each do |filter|
if(filter.is_a?(ActionController::Filters::BeforeFilter) and filter.method == before_filter_name)
return filter
end
end
end
return nil
end

#asserts a single BeforeFilter is applied on a single action
def assert_before_filter_applied(filter, action)
if(filter.nil? or action.nil?)
assert false
end

if(filter.options.empty?)
assert true
return
end
if(!filter.options[:only].nil?)
assert filter.options[:only].include?(action.to_s)
end
if(!filter.options[:except].nil?)
assert !filter.options[:except].include?(action.to_s)
end
end

#asserts a single BeforeFilter is not applied on a single action
def assert_before_filter_not_applied(filter, action)
if(filter.nil? or action.nil?)
assert false
end

if(filter.options.empty?)
assert false
end

if(!filter.options[:except].nil?)
assert filter.options[:except].include?(action.to_s)
end
if(!filter.options[:only].nil?)
assert !filter.options[:only].include?(action.to_s)
end
end
end
Now my test code looks like the following-
  def test_filters
should_apply_before_filters(:authenticate, :restrict_invalid_role)
end
I can use the other methods as well to get finer control if the before_filter is applied using :only or :except options. And I can use these helper methods across all my controller test classes, making the whole point of testing filters really neat and short.

If you are familiar with shoulda tests, you will find the above code following shoulda like naming conventions. I found the above code to strip a lot of your efforts, since you eliminate all test codes that safeguard your filters.

Hope it helps someone with similar need.

Sunday, April 26, 2009

Forget Me Not! Object Oriented Design in Custom Exception

When designing custom exceptions, you may forget about old school OO fundamentals. As a reminder, lets take a look into the following custom exception classes.


StorySaveError
StoryDescopeNotAllowedError
StoryCompleteError
StoryNotFoundError
StoryDeleteError
StoryDeleteNotAllowedError

These exceptions are really useful in my application. But the bad thing is, they all derive from StandardError class, whereas there should be a base class, may be StoryError, which is more meaningful and useful. So, we can have the following-


class StoryError < StandardError
end
StorySaveError < StoryError
StoryDescopeNotAllowedError < StoryError
StoryCompleteError < StoryError
StoryNotFoundError < StoryError
StoryDeleteError < StoryError
StoryDeleteNotAllowedError < StoryError

With the addition of a base class for all Story related exceptions, I can write cleaner/better code.

  • When I am not interested about the specific error, I can just use rescue StoryError to catch 'em all
  • I can place a common error handling routine for all Story related exceptions
  • I can add new exception types without altering the codes where the specific type of StoryError is insignificant

From my experience, I found that most people are not cautious about OO when desiging custom exceptions. (because they are exceptions!). Nonetheless, if you follow the OO guides, it will pay off for sure.

Wednesday, April 15, 2009

Implementing Template Method in Rails Controllers Using Module and Mixin

All rails controllers are subclasses of the ApplicationController class. A typical controller class declaration will look like the following-

class LoginController < ApplicationController
#the actions go here
end

With this basic information, I would like to state the problem first.


The Problem/Job in Hand

Add an action switch_project to all the controllers (> 20) of the ScrumPad code base. The implementations of the switch_project method will be same for all the controllers only other than the fact that, the switching destination will be different.


Analysis

Placing the switch_project action in the ApplicationController would be the best option. But, the methods of application controller are not accessible as actions. So, the following won’t work


class ApplicationController
def switch_project
if(is_valid_switch?)
logger.debug(“A switch is taking place”)
destination = get_destination(new_project_id)
redirect_to destination
end
end
end

if you hit http://server/login/switch_project you will get a server side error. However, if you instead place the switch_project inside the LoginController, it will work fine. But, of course at a cost. You need to copy/paste this method 20+ times as there are 20+ controllers with the same need! Horrible!


Again, as I said, the get_destination(new_project_id) is the only part that will be different for each of the controllers. So, we definitely find a template method here.


The Solution

If you need an introduction about Module and Mixin in Ruby, please read here at ruby-doc. We are going to use Mixin to implement the desired solution, efficiently.

So, I put the switch_project method in a module called, ProjectSwitchModule inside a new file at app/controllers/project_switch_module.rb like this-


module ProjectSwitchModule
def switch_project
if(is_valid_switch?)
logger.debug(“A switch is taking place”)
destination = get_destination(new_project_id)
redirect_to destination
end
end
def is_valid_switch?
#I determine if the switch is valid at this method and return boolean
end
end

To make it available to all my controllers, I include this module in just in the ApplicationController in the following way-


require ‘project_switch_module’
class ApplicationController
include ProjectSwitchModule
end

Also, to provide controller specific implementation of the get_destination(project_id) method, I am just writing this method in each of the controllers in the following way-


class LoginController
private
def get_destination(project_id)
#custom destination logic for LoginController
end
end
class MessageController
private
def get_destination(project_id)
#custom destination logic for MessageController
end
end

Now, if I invoke http://server/login/switch_project or http://server/message/switch_project I will get the result of the switch_project action. So, this gives us an elegant way to follow design patterns for efficient implementation. It will save a lot of my time in future when I need to change the switch_project method, since I just need to change at a single place instead of 20s.


Afterthoughts

If, for a controller the switch_project needs to be different from the template at the module, it is achieved simply by overriding the switch_project inside the controller. No worries!


I will appreciate any feedback on this article.

Wednesday, August 13, 2008

Unit Testing using Mocks - FillWithMocks, Fill all or only selected properties with Mocks using only a single method call

I have been doing TDD for about two years now and using mock testing for interaction based unit testing in my projects. What I have learned over this time is, a unit testable design leads to introduction of interfaces and dependency injection for testing a code in isolation. And when I want to perform tests on my interactions, I need to create mock objects and inject these mock instances to my object under test. Sometimes, a unit test class needs to create quite a few of such mock objects and I feel this can be done using a simple wrapper around the usual mocking frameworks.

I suggest a similar and even more powerful wrapper so that you don't need to create instances for each of the mock objects, rather do it in a single call for all your desired mocks. I have shown this method for NMock2, however, its evident that you can write your own method for your favorite mocking framework just using this code as a reference.

This code is written in C# 3.0 and should compile in .Net 3.5. You will need to add a reference to NMock2 to compile this. Also, you need to know a bit about Reflection to understand the following code fragment.

Disclaimer: This code is just a simple example and it may not suit all your needs.

1 using System;

2 using System.Collections.Generic;

3 using System.Linq;

4 using System.Text;

5 using System.Reflection;

6

7

8 namespace NMock2.Extensions

9 {

10 public static class MockeryExtensions

11 {

12 /// <summary>

13 /// This method will fill all properties of the 'target' of interface type having a setter

14 /// </summary>

15 /// <param name="mocks"></param>

16 /// <param name="target"></param>

17 public static void FillWithMocks(this Mockery mocks, object target)

18 {

19 Type targetType = target.GetType();

20 PropertyInfo []properties = target.GetType().GetProperties();

21

22 foreach (PropertyInfo property in properties)

23 {

24 Type ptype = property.PropertyType;

25 if (ptype.IsInterface)

26 {

27 if(targetType.GetMethod(string.Format("set_{0}", property.Name)) != null)

28 {

29 targetType.GetProperty(property.Name).SetValue(target, mocks.NewMock(ptype), new object[] { });

30 }

31 }

32 }

33 }

34

35 /// <summary>

36 /// This method will fill the properties with names specified in the propertyNames array having a setter

37 /// </summary>

38 /// <param name="mocks"></param>

39 /// <param name="target"></param>

40 /// <param name="propertyNames"></param>

41 public static void FillWithMocks(this Mockery mocks, object target, params string[] propertyNames)

42 {

43 Type targetType = target.GetType();

44 foreach (string propertyName in propertyNames)

45 {

46 PropertyInfo pInfo = targetType.GetProperty(propertyName);

47 if (pInfo != null && targetType.GetMethod(string.Format("set_{0}", propertyName)) != null)

48 {

49 pInfo.SetValue(target, mocks.NewMock(pInfo.PropertyType), null);

50 }

51 else

52 {

53 throw new ArgumentException(string.Format("the property {0} is not found or does not have a setter", pInfo.Name));

54 }

55 }

56 }

57 }

58 }

So, with this extension method in place, you can go and write your test code in a much compact manner. The following is an example showing one possible use of this method in your NUnit test code.

83 public interface IMyInterface

84 {

85 void MyMethod(string name);

86 }

87

88 public class MyExampleClass

89 {

90 public IMyInterface MyPropertyOne

91 {

92 get;

93 set;

94 }

95

96 public IMyInterface MyPropertyTwo

97 {

98 get;

99 set;

100 }

101

102 public IMyInterface MyPropertyThree

103 {

104 get;

105 set;

106 }

107

108 public MyExampleClass()

109 {

110

111 }

112 }

113

114 [TestFixture]

115 public class MyExampleClassTest

116 {

117 private MyExampleClass _myExampleClass;

118 private Mockery _mocks;

119

120 [SetUp]

121 public void Init()

122 {

123 _myExampleClass = new MyExampleClass();

124 _mocks = new Mockery();

125

126 //This call fills all the three properties with mocks

127 _mocks.FillWithMocks(_myExampleClass);

128

129 //This call fills only MyPropertyOne and MyPropertyTwo with mocks

130 _mocks.FillWithMocks(_myExampleClass, "MyPropertyOne", "MyPropertyTwo");

131 }

132 }

Comments are welcome!

Sunday, August 03, 2008

Ninject - Dependency Injection Framework for .Net Objects

I loved Ninject because

- I don't love to write/edit XML documents myself.

- I think Ninject can save a lot of coding and debugging time.

- This is a cool framework :-) with a fun-to-read documentation.

- It uses simple conventions and Attributes for resolving most dependencies behind the scene.

- You can take full advantage to lambda expressions for writing compact codes.

Today, I took a look into this Ninject framework. I have used Spring.Net and Unity in my previous and current projects. But seems like Ninject has the most 'fluent vocabulary' that lets you develop on most 'fluent interfaces'.

Take a look at the fun-to-read documentation at http://dojo.ninject.org/wiki/display/NINJECT/Home and enjoy the 20 minutes while you eat the whole document!

Tuesday, March 18, 2008

Verify Correct DateTime data in method call using NMock

Recently I was writing unit test for a method of the following signature

interface ICache
{
void
Insert(string key, object value, DateTime absoluteExpiration, TimeSpan slidingExpiration);
}

and I wanted to verify the following call -

Insert("key", "value", DateTime.Now.AddMinutes(30), TimeSpan.Zero);


However, you readily see the problem in verifying with the above call for the presence of
DateTime.Now.AddMinutes(30) in the argument. So, a test method like the following won't work for obvious reason.
ICache cache = _mocks.NewMock<ICache>();
Expect.Once.On(cache).Method("Insert").With("key", "value", DateTime.Now.AddMinutes(30), TimeSpan.Zero);


The reason that the above sample code wont work is, my test code called DateTime.Now.AddMinutes(30) before the production code actually did it. So, it is highly probable that the two version of DateTime.Now.AddMinutes(30) in the test code and production code are not same.

To find a work around to this problem, I changed my assumption a little. I set my assumption that the value for the '
absoluteExpiration' parameter must lie in between 29 and 31 minutes from now. So, I modified my test method like the following call-

Expect.Once.On(cache).Method("Insert").With("key", "value", Is.NotNull & Is.AtLeast(DateTime.Now.AddMinutes(29)) & Is.AtMost(DateTime.Now.AddMinutes(31)), TimeSpan.Zero);

This advanced level of NMock usage may show you a way in similar needs. For more insight to this solution, you may wish to visit http://nmock.org/advanced.html



Sunday, March 16, 2008

Mock Internal Interface with NMock2 - Use InternalsVisibleToAttribute

Yesterday I badly needed to create mocks for my internal interfaces. I am using NMock2 as a Mocking framework and found it really difficult to produce Mocks for internal interfaces. After some 'google time' and investigation I found that you need the following two lines in your AssemblyInfo.cs to achieve this purpose-

[assembly: InternalsVisibleTo("Mocks")]
[assembly: InternalsVisibleTo("MockObjects")]

Where are the two assemblies? I found that these two assemblies are dynamically created by NMock2 for mocking your interfaces.

To find out the truth yourself, just put a breakpoint and watch the value of the following line-

AppDomain.CurrentDomain.GetAssemblies()

You should see the two assemblies in the list of the assemblies in the current application domain.

I found it interesting. I wish someone of you might find it useful too!

Note: You may still need to add InternalsVisibleTo attribute for your test assembly with the above two declarations.

Saturday, March 15, 2008

Unity - A Dependency Injection framework from Microsoft Patterns and Practices

I have been using Spring.Net for implementing Dependency Injection in my .Net projects. Spring is already one of the most known providers of DI in the Java world and a port to .Net actually eased the lives of people like me who are willing to develop unit-testable software.

However, with all the happiness enjoyed through Spring.Net, I guess its time to take a look into Unity. I have only started using this and I found it interesting in the sense that its more of .Net flavored with a few attributes and fluent interfaces.

I hope to post a few concrete examples on Unity implementation soon. However, you may wish to take a look at http://www.codeplex.com/unity in the meanwhile and let me know your comments on this new application block by Microsoft Patterns and Practices group.

Sunday, December 02, 2007

Unit Testing void Methods - Part 1

Dissection of void Methods without parameters

Methods with void return types incur complexities in writing unit tests. So, we need to characterize the void methods to make sure we have guards against the odds for testability.

A typical void method without any parameter looks like the following

public void DoSomething()
{

//1.modifies some member variables
this.someMemberVariable = 10;

//2.Calls methods of the same class or other classes
someClass.SomeOtherMethod(someArg);

//3.Sometimes throws exception
}


So, we need to make sure we can test all the activities performed by such a method. The following are the illustrative examples showing the unit test solutions case by case

a. To test a method that modifies a member variable we need to write assertions on the member variables accessors. If this is a private member without a getter method then we will overlook it for the same reason as we discard the testing of private members

Note: Examples coming up shortly.

b. If this method calls a method from this class, we need to make sure that the called method is tested. If the called method belongs to anther class then, we can use dependency injection to use a mock for this method or make sure this method is testable.

Note: Examples coming up shortly.

c. If a method throws exception on some certain conditions, we can test for that exception with the appropriate conditions.

Note: Examples coming up shortly.