Show – Off always Shown up in Showdown
This was my fortune cookie the other day. It was attributed to Confucius. Now, I doubt that Confucius actually said it. The humor shines through only in English, I think. For the sake of not plagiarizing, let’s continue attributing it to Confucius. It reminds me of another quote that I heard a while back. This one attributed to a Buddhist monk “What I am I am and say not. Being is the great explainer”. We will get to this quote in a moment.
But first …
If you are a job candidate hoping to make it past the screening process, you may well feel like a show-off. Regardless of your personality, the job interview is not the time for modesty. The job interview is the time to say and with a straight face “Hey look at me. Aren’t I wonderful? Wouldn’t you like to have me working for you?”
If you are looking for a job, depending on how you find yourself looking, this can be tough. If you got fired (laid off, downsized, cost reduced, name your euphemism) such confidence will feel alien and fake. If you are miserable in your current job and casually looking around, you aren’t really motivated to put it all out there, and you can’t maintain the strong self confident image that you need to present.
Fake or uncomfortable, awkward or contrived, however it makes you feel, this is the façade that you have to put on to get the job.
Most of us technology geeks will more easily tend towards the Buddhist quote listed earlier. We would rather not have to toot our own horn. We would rather be modest and casual keeping a low profile and avoiding the spotlight. We try to live by the second quote mentioned earlier.
“What I am I am and say not. Being is the great explainer”. If you are a great programmer, it should be obvious if you are doing your job right. As a general rule, if you have to walk around saying, “I am great at this” or “I am great at that” you usually are not. More likely than not, you are an insufferable jerk. If you really are great, it should be obvious and you should not have to point it out all the time. This is a great philosophy for the daily grind.
Unfortunately, it doesn’t work so well for job searching. In a short interview, you flat out don’t have the time for “being to explain” what you are, must shine through quickly, usually in 30 minutes or less. If not, you may not get the rest of the time needed for being to explain.
So, if you are on the job hunt, let loose your inner braggart. Just me sure to be him back in the cage after you land your next great opportunity. If you are the screener, realize that the people you are dealing with are probably acting against their character and venturing outside their comfort zone. Be patient.
Thoughts on a Data Dictionary
I have often thought about what would make a good data dictionary. Usually I have lamented the absence of any meaningful data dictionary beyond what the DBMS itself keeps on its own. If everyone would simply add comments everywhere that the database natively allows, we would all be oh so much better off. But alas we don’t.
As is so often the case, if we don’t have to do something, we probably won’t. Often times, it’s a matter of meeting deadlines, sometimes it’s a matter of shear laziness, but sometimes, it may just be a case of not understanding the need or not knowing what is intrinsically available.
Let’s tackle the need first…
Database objects rarely have meaningful names. Database objects with meaningful names to one group may not be meaningful to another group. When you are trying to learn a new data model, the history of this data model is useful to understand the conventions adopted or ignored.
In its simplest form, I have several goals for a data dictionary:
· Define the naming conventions followed
· Identify known exceptions to these conventions
· Provide an English description for each database object (table, column, stored proc, trigger, etc)
· Document the accepted data types and their purposes
· Identify known exceptions to the data type rules
In Oracle or SQL Server, this can all be accomplished with a relatively small word document and the native data dictionary.
I think though that to actually enforce these conventions, we need a little extra support:
· A Data Thesaurus
· A rule validator similar to fxCop
The data thesaurus would be used to ensure that appropriate naming conventions are used. Proposed object names would be split on a case change and run through the thesaurus. If the individual words are found in main line entries, you are good to go. If an individual word is found as an alternate, replace it with the main line entry and proceed. If an individual word is not found in the thesaurus, flag it for review.
Such a process would eliminate such confusing names as:
· ProcessDate
· ProcessDte
· Process_Date
The first name would work as entered. The second one would be converted to the first one, and the third entry would be rejected since Process_ was not found in the thesaurus. The key thing here is that it tracks and enforces the convention at the time of object creation not at the end of the process.
The above is one rule based on the data thesaurus. Other rules that may be useful might include:
· Insure that object names are not plural. Each record in the “Members” table is a Member
· Insure that each table name is a noun
· Ensure that every column is either a noun or an adjective. Columns should not be verbs
· Ensure that every table has an associated comment
· Ensure that every column has an associated comment
· Ensure that every stored procedure has an associated comment
· Ensure that stored procedures pass the thesaurus test
· Ensure that stored procedures have the form ActionObject, such as InsertMember or SelectMembersByGroupID
Or
· Ensure that stored procedures have the form NounAction such as MemberInsert or MemberSelectByGroupID
What are your thoughts on a data dictionary? Has anyone incorporated similar ideas into one? How did it work out? Have you thought about and decided against?
Model View Presenter / Passive View Benefits
A hot topic near and dear to my heart is Model View Presenter (MVP) or Passive View design pattern. This design pattern forms a good model for keeping a strong separation between UI logic and business logic.
In the MVP model, the data access objects constitute the model, the View is the UI components kept as simple as they can be and the Presenter is where the business logic is. These three components interact with other through interfaces to further reinforce the logical separation.
The presenter should have no idea where the Model gets its data or how the data is persisted. The Model defines an interface that must be implemented. Anything that implements this interface can serve as the model. This means that we can switch out the data storage strategy without having to change anything in the presenter.
The presenter should also have no idea about the nature of the View. The View implements an interface that exposes all of the methods, properties, and events that the Presenter will need and the Presenter implements the business logic through this interface. This means that View may actually been a Win Form, Web Form, a server control, a user control, a SharePoint web part, or even a class library with no UI components. The only requirement is that the View must implement the interface.
Among other things, this gives us these benefits:
· Simplified testing. The Presenter can be tested without having to worry about the database or the UI. Both the Model and View can be replaced with “Mock” objects designed to simplify testing
· Business logic implemented in the Presenter can easily be retargeted to any number of UIs. Once the business logic is implemented and tested, the original View can be converted from a Win Form to Web Form or a Mobile Web Form, etc without impacting the Presenter as long as the new Views continue to implement the same interface
· The Model can be switched out to for demo purposes to read from local flat files or static XML files. As long as the new Models implement the original interfaces, neither the Presenter nor the Views needs to change.
· If you have multiple lines of business with similar business logic but very different data sources, new Models can be implemented targeting the new data sources. As long as the new models implement the original interfaces, neither the Presenter nor the View needs to change.
Future posts will explore what some of these benefits mean as well as ferret out some additional benefits.
What benefits do you see with the MVP pattern? What hurdles have you had to face?
What can we do for meaningful variable names?
You hear much about naming conventions and some shops still have some arcane rules in place. Fortunately most of us have stepped beyond Hungarian notation. I have seen some shops define rules that the method name should include the number of parameters. I have seen shops define rules that would ban overloading and insist on a sequence number for methods that should be overloads. I have seen shops that wanted the Requirement Number embedded in method names, and I have seen shops put maximum character size limits to simplify reading. This unfortunately led to awkward abbreviations.
None of these standards have led to more meaning variable names. The fact that a variable is an integer or a string does nothing for me. Having a method signature come like:
Public void int DoSomething (string strParam, int intParam)
does nothing for us. Having one like:
Public void int ProcessIncomingClaims (string branchCode, int transactionID)
Can be very enlightening.
Like legislating morality, none of these rules will guarantee that the names are good. At the same time, almost any set of “good” rules could still be applied and end up with “bad” variable names. We may not always know when a name is “good” but we usually know when the name is “bad”.
Enter GhostDoc. GhostDoc is a wonderful tool designed to help automate your comments. Caution it works best for C#, but they are building support for VB. The intention here is that GhostDoc takes your method, properties, parameters, etc. and splits them to come up with the documentation for that item. There are several thinks that seem to be happening here. First it splits the words at a case change. Second, it interprets methods as being in the form verb – object and will attempt to structure the comment like that.
For instance, if you have a method called DataBind (), GhostDoc will come up with the comment:
/// <summary>
/// Datas the bind.
/// </summary>
This is clearly not what we want for a comment. Note that the naming convention was not followed. We would want to have methods following the pattern of Vern Object. Here it is following the convention of Object Verb. Ghost Doc ends up with a comment that does not make sense grammatically. Now consider the comment generated for the method BindData ():
/// <summary>
/// Binds the data.
/// </summary>
Our method name follows the naming conventions that we have setup and now the generated comment makes sense as well.
Try it and you will find that whenever GhostDoc does not come up with a boiler plate comment, your variable names or method names or property names are probably not named well.
Not a bad litmus test for good names!
Just like Ghost Writers in the publishing world, there is only so much that GhostDoc can do. It does a good job of structuring your comments. It does a good job of mapping out the details needed for comprehensive XML documentation. It also does a good job brining in ancestor details when overriding from a base class.
It does not explain how logic works or why logic is needed. This you still need to add yourself, but at least you will know where it goes to produce in comprehensive documentation. The better you name your methods and parameters, the less you should have to write. The more highly cohesive your methods, the less you should have to write. The less you have have to write manually, the better your code can be understand without your comments.
Has anyone else used GhostDoc to help ensure naming conventions or help ensure that code is readable?
?
You hear much about naming conventions and some shops still have some arcane rules in place. Fortunately most of us have stepped beyond Hungarian notation. I have seen some shops define rules that the method name should include the number of parameters. I have seen shops define rules that would ban overloading and insist on a sequence number for methods that should be overloads. I have seen shops that wanted the Requirement Number embedded in method names, and I have seen shops put maximum character size limits to simplify reading. This unfortunately led to awkward abbreviations.
None of these standards have led to more meaning variable names. The fact that a variable is an integer or a string does nothing for me. Having a method signature come like:
Public void int DoSomething (string strParam, int intParam)
does nothing for us. Having one like:
Public void int ProcessIncomingClaims (string branchCode, int transactionID)
Can be very enlightening.
Like legislating morality, none of these rules will guarantee that the names are good. At the same time, almost any set of “good” rules could still be applied and end up with “bad” variable names. We may not always know when a name is “good” but we usually know when the name is “bad”.
Enter GhostDoc. GhostDoc is a wonderful tool designed to help automate your comments. Caution it works best for C#, but they are building support for VB. The intention here is that GhostDoc takes your method, properties, parameters, etc. and splits them to come up with the documentation for that item. There are several thinks that seem to be happening here. First it splits the words at a case change. Second, it interprets methods as being in the form verb – object and will attempt to structure the comment like that.
For instance, if you have a method called DataBind (), GhostDoc will come up with the comment:
/// <summary>
/// Datas the bind.
/// </summary>
This is clearly not what we want for a comment. Note that the naming convention was not followed. We would want to have methods following the pattern of Vern Object. Here it is following the convention of Object Verb. Ghost Doc ends up with a comment that does not make sense grammatically. Now consider the comment generated for the method BindData ():
/// <summary>
/// Binds the data.
/// </summary>
Our method name follows the naming conventions that we have setup and now the generated comment makes sense as well.
Try it and you will find that whenever GhostDoc does not come up with a boiler plate comment, your variable names or method names or property names are probably not named well.
Not a bad litmus test for good names!
Just like Ghost Writers in the publishing world, there is only so much that GhostDoc can do. It does a good job of structuring your comments. It does a good job of mapping out the details needed for comprehensive XML documentation. It also does a good job brining in ancestor details when overriding from a base class.
It does not explain how logic works or why logic is needed. This you still need to add yourself, but at least you will know where it goes to produce in comprehensive documentation. The better you name your methods and parameters, the less you should have to write. The more highly cohesive your methods, the less you should have to write. The less you have have to write manually, the better your code can be understand without your comments.
Has anyone else used GhostDoc to help ensure naming conventions or help ensure that code is readable?
Reflecting on Software Metrics
At the risk of sounding pedantic, I like software metrics. They can prove to be invaluable in analyzing source code. To be clear, I am not proposing that developers be bonused based on metrics and their use in estimating and scheduling should be limited at best. I am however a fan of software metrics as benchmarks for evaluating design and understanding how software works.
There is an add-in for reflector called CodeMetrics that allows you to easily calculate a wide range of code metrics from within Reflector. We are most interested in one metric, Cyclomatic Complexity.
Cyclomatic Complexity or CC is the number of logical paths through a piece of code. This can be used to define a lower bound for the number of test cases needed to ensure proper coverage in unit testing. This can also be used to predict ongoing maintenance costs and be scaled to predict the likelihood of actually resolving a particular defect or introducing additional defects.
Tables similar to this are common:
|
CC
|
Testing Requirements
|
Ongoing Maintenance Costs
|
Resolution Likelihood
|
|
1-0
|
A simple procedure
|
Low
|
High
|
|
11-50
|
Complex
|
Moderate to High
|
Medium to low
|
|
>50
|
Very Difficult
|
High Risk
|
Low
|
The actual bands may vary depending on the line of business and skills of the developers involved, but these are generally good benchmarks.
A CC of 10 would indicate that 10 test cases are needed to fully test all of the functionality. Depending on what you are working on, that may be more complex than you want to consider or that may still be fairly trivial. As a general rule, I try to keep methods below 10.
When I am working on existing functionality, I try to review the code and refactor the code in the same module with the highest complexity, systematically reducing the overall complexity. Sometimes the best you can do is to review the more complex functions and add comments on how they may be refactored. Often times, the actual refactoring may be out of scope for the change that got you involved in the first place.
It is important to review the complexity of any code before making code changes. If you are about to change code that already has a high complexity, you want to make sure that your changes do not add more complexity. You need to be aware that with higher complexities comes a higher chance of introducing new defects and you are also less likely to resolve the underlying defect if it is embedded in complex code. This may change your estimates and should change your testing strategy.
Sometimes, complex code masks cohesion problems. If the code is very complex, there is a good chance that the particular method is trying to do more than one thing. If it is doing more than one thing, than simplifying the code may be nothing more than separating all of the different tasks that the method is trying to do. For example, code is often complex because data validation logic is blended with business logic or data mapping logic is blended with business logic. Simply pulling all of the data validation logic to a separate method and all of the data mapping logic to a separate method and isolating all of the business logic to a single method, you end up with several methods each of which is less complex than the original. Each of these methods will be much more cohesive. With cohesive methods, it is easier to know where to add new functionality or where to go when reviewing existing functionality.
Future blogs will explore some ways to reduce complexity.
Passive View and Cyclomatic Complexity
We have previously talked about cyclomatic complexity and set forth the goal of keeping the complexity for our methods down below 10. We have also talked about the goal of systematically refactoring any methods that we find to lower complexities whenever we make code changes. These are good and admirable goals for business logic.
But what about Passive View? Passive View is a slight variation on the Model View Presenter pattern designed to drive home the fact that the view should do very little processing. The view should be passive. Quiet literally the only code found in the view should be the implementation of the properties and the event handlers for the UI components. The properties should do the absolute minimal to interact with the user, and the event handlers should simply forward calls on to the presenter.
I cannot stress this enough, there should be no actual logic in the view.
The driving force behind the Passive View pattern is to simplify testing. Since it is harder to test the UI and the UI does not lend itself to automated testing, we want to move as much logic as possible to locations that are easier to test, class libraries. Testing aside, logic housed in class libraries is more reusable and can be shared across multiple UIs. In fact, the automated tests simply become a new UI.
All that being said, any method in the UI should have a cyclomatic complexity of 1!
Yep! You heard me correctly. One! If you find yourself writing a method, event handler or property in the UI with a complexity greater than one, stop and ask yourself why whatever logic you are implemented is in the UI. Chances are you will find that there is functionality that should be moved to the Presenter or a shared UI component.
I challenge everyone to identify UI code with a complexity greater than 1 that cannot be moved out of the View. Note that I said out of the View and not out of the UI layer. There are times when true UI processing may require some logical processing, but these are not so specialized that they cannot be removed from the View.
Using Pronouns
This may not be very technical, but hopefully it may generate some discussion and provide some insight.
Pronouns can be very enlightening about our attitudes towards work. How long do you work for a company before you start thinking of the company as “we” instead of “they?” Consider: We are the number 1 producer of product X. Vs. They are the number 1 producer of X. Do you ever make that pronoun switch? What does it mean if you switch back from “we” to “they?” What does it mean if you never make the switch? Are you even aware of which one you use?
The dynamics with interpersonal relationships are also interesting. Have you ever noticed that a single co –worker will probably say “I” or “my”, or “me” a lot. A co- worker married or at least in a committed relationship will be much more likely to say “we” even if their partner is not with them. What should we make of this? : )
Of course you also get this with team dynamics as well. A strong team player will frequently refer to “we.” In this case, “we” probably refers to the team. We are falling behind and need to catch up. In a strong team, “I” rarely makes an appearance. Hopefully, “you” is scarce as well.
I have also noticed something interesting with my wife that I thing is somewhat common. The switch from “our” or even “mine” to “your” when we get upset about something. I notice “our” dog becoming “your” dog when the dog does something bad. Is this common? Does it show up in other areas of life? Do “our” processes become “your” processes when we feel like the processes don’t work? Would such switches be a Freudian slip, malicious behavior, or a harmless word game? Are we even of it when we make such switches?
So your use of pronouns can be used gauge the ties you feel towards your employer, your team, and your partner. Here are some of my thoughts:
· When your employer becomes “we” as in “We have a good work environment” instead of “They have a good work environment”, you finally feel like you are part of the corporate culture.
· Should you go back to thinking of your employer as “they”, you may be feeling a little disenfranchised or disconnected. Consider, “They have a strong business model” instead of “We have a strong business model”
· If you never think of the company as “we,” are you perpetually disconnected looking in on your co-workers from the outside?
· When you meet someone and they keep referring to “We went to the movies over the weekend”, chances are that person is married. If your new friend states that “I went to the movies over the weekend,” you may have a new single friend.
· When your team mates start referring to “we” as in “We have a good sense of the project timelines”, you probably have a strong team. As long as you are still thinking in terms of “I think I know what needs to be done”, your team probably hasn’t truly formed yet. Or maybe it is still “storming”
These are not hard and fast rules. These are probably not even generally guidelines. These are simple observations that I have made that seem to be true often.
Without a doubt there are many exceptions. Most of us choose our pronouns without even thinking about it. If we start thinking about it too much, we will probably throw the whole system out of balance, but a little self reflection is a good thing.
I was recently challenged by a very good friend that the contnet of my blog was only really clear to people who did not need to read it. His comments challenged me on several fronts.
- Why do I post blogs?
- Who do I believe my target audience is?
- What do I hope this audience gets from the blogs that I post?
There are several reasons why I post blogs. Sometimes I post a blog so that I can quickly find something that I struggled to figure out in the first place. In this case, the target audience is myself, and I hope to make it easier for me the next time that I do something similiar. If others find something useful there, that is great, but they are not necessarily the target audience. I admit that these blogs are rare, or atleast I hope that they are rare. Probably more blogs are written with me as the target audience than I intend.
Sometimes I post blogs with the intention of giving back to the community and helping others have an easier transition in learning something new, or encouraging someone to venture into new technologies that they may otherwise be intimidated by. In such cases, the target audience is someone new to DotNet or someone who has not really explored much beyond what was needed to get specific jobs done. This is where I probably have failed my audience the most in the past. The content may not always be as readily accessible as I would intend.
Sometimes I post blogs with more of a "Look at me, ain't I smart" attitude. I am ashamed to admit that this is more common than I wish and probably leads to the problems with many posts not being easily accessible to who I believe my target audience is. I challenge anyone reading my blogs to help keep me inline. Just be nice about it.
My friend recommended a format that he thought would make the content more accessible to a broader audience by focusing on:
- What does this mean
- How will this help me
- How do I get started
This sounds like sound criticism and good advice. I challenge all of you who do post blogs to think about the three questions that I struggled with at the begining.
- Why do I post blogs?
- Who do I believe my target audience is?
- What do I hope this audience gets from the blogs that I post?
How would you have to answer these questions? What do you think of my firend's suggested format?
Debugging Attributes
Have you ever been debugging code and examined the properties of an object? You get both the public properties and the private member variables! Why is that? Can this be avoided?
This seems to violate the principles of encapsulation. It also complicates the process of examining the contents of an object at run time. You don’t really want to have to look at all of the private member variables which by definition should be irrelevant to the task of debugging.
Fortunately the violations to encapsulation pose limited risk. Even though the debugger will reveal hidden member variables, you still cannot access them in your code.
The confusing part is when you view an object’s detail and you have to filter through potentially twice as many variables as there are properties.
How do you remove this potential confusion in the objects that you create? Enter the System.Diagnostics.DebugBrowsable attribute:
Add a DebuggerBrowsable attribute to private member variables.
[System.Diagnostics.DebuggerBrowsable (System.Diagnostics.DebuggerBrowsableState.Never)]
Now these variables will not show up when you do a Quick Watch. This will clean out the entries from the Quick Watch that you did not intend to be visible.
Another debugging frustration is stepping into methods or properties that you are no longer interested in.
Before c# 3.0 introduced implied properties, business entities could be particularly frustrating since they often included several very simple properties that you never had any need to step into.
I would often try to step into a method and have to step into several properties’ get accessors to pass their values as parameters.
Not only is this annoying, it is also potentially confusing and time consuming as the debug bounces around in irrelevant code as you are trying to figure out why an errant method is not behaving as you expect.
If only there was a way to instruct the debuger to ignore certain pieces of code …
Turns out that there is!
[System.Diagnostics.DebuggerHidden] Causes the debugger to completely ignore the method.
[System.Diagnostics.DebuggerStepThrough] The debugger will not step into the method but will honor breakpoints in the method.
[System.Diagnostics.DebuggerNonUserCode] This attribute is similar to combining the first two attributes. This is intended for generated code or designer code.
Since these are attributes, you have to pay attention to the AttributeUsageAttribute.
DebuggerHidden and DebuggerNonUserCode can be tied to properties. DebuggerStepThrough cannot. It can be applied to methods though. This means that it would have to be applied to the get and set individually for a property and not to the property as a whole.
The DebuggerStepThrough attribute causes some problems for CodeDom since there is no way to associate attributes with the individual methods that make up a property. Use the DebuggerNonUserCodeAttribute instead.
ConditionalAttribute
Certain methods especially in the System.Diagnostics namespace are decorated with ConditionalAttributes similar to this:
[Conditional("DEBUG")]
What is this all about? The conditional attribute provides a nice way to have code be conditionally ignored. Methods decorated with this attribute will not have their calls compiled to IL unless the condition is true.
In the attribute shown above, any calls to the method will not be made unless the symbol DEBUG is defined.
This creates some very intriguing possibilities. Some of which might be nice and some of which might be pure evil.
Code like this:
#define TEST
using System;
public class MyClass
{
public static void Main()
{
#if (TEST)
Console.WriteLine("TEST is defined");
#endif
}
}
Can be rewritten to code like this:
#define TEST
public class MyClass
{
public static void Main()
{
TestWriteLine("Test is defined");
}
[System.Diagnostics.Conditional("TEST")]
public static void TestWriteLine(string message)
{
Console.WriteLine(message);
}
}
Is this better? Consider this, you would have to wrap every method call the “if test compiler directives” to achieve the same effect of adding the attribute to one place.
If you are only calling the method from one place, this probably doesn’t matter, but if you are potentially calling this method from throughout multiple enterprise applications, this is unrealistic.
The compiler directives can cause confusion by interrupt the logic flow.
Here are some examples where this might be very useful and practical
Suppose you wanted to be able to support a build that could be used for code coverage testing without having to change code for a production build.
Suppose you wanted to have a build to support performance metrics without having to change any code.
Suppose you wanted to have a build to support debugging information without having to change production code.
Turns out, this last example is the whole reason this attribute was created. This is how Debug.WriteLine works.
Try this, sprinkle your code with calls System.Diagnostics.Debug.WriteLine(), compile in debug mode, and examine the assembly with Reflector. You will notice that all of your calls to WriteLine are there. Now compile in release mode and examine the assembly with Reflector. You didn’t change any code, but now none of the WriteLine statements are there.
We could create a method decorated with a conditional attribute requiring that the symbol CodeCoverage must be defined and then call this method at the start of every function. Compile the code defining this symbol. Now when we run through a round of testing, we will get a list of every method that was called. Recompile the code without defining this symbol and the overhead of logging function calls goes away.
As is often the case, there are a couple of caveats. This is not documented on MSDN anywhere that I have seen. Any method decorated with this attribute must return void and cannot have output parameters. This is intended to prevent the removal of this code from causing any unintended side effects. You would not want to rely on a variable being initialized by a function that may or may not be called.
How could you envision using such attributes?
The refactoring book http://www.amazon.com/exec/obidos/ASIN/0201485672 is really one of the best books that I have seen on programming. If you are new to programming, I highly recommend giving the book a read. Learning some of these refactors and thinking about them as you write code will help you write better code. Being familiar with the various refactors will help you quickly identify ways to improve code. The concept of code smells is pure genius.
The concept of refactoring is very liberating and freeing. You don’t have to worry about getting everything right the first time. You never will. Refactoring provides a general framework for not only reversing the inevitable deterioration of good code into bad code, trust me it happens. It also provides a framework to move code that started out not so good into better code.
No one sets out to write bad code, but still it happens.
Sometimes requirements change during development.
Sometimes requirements are not well understood leading to a bad design from the outset.
Sometimes you simply run out of time and do the best you can knowing that it is not as you would like to write, but having to do it anyway.
Sometimes you are learning a new technology and simply don’t know any better. It’s dangerous writing software while best practices are still emerging.
Sometimes multiple people are working on the same thing and not everyone understands the design, assuming that there was an original design.
Sometimes new functionality is “bolted on” awkwardly.
Sometimes incremental changes skew the code base in awkward directions.
There are many reasons bad things can happen to code software. When you see software that has gone bad, be careful casting blame. There is no telling how bad your code may look over time. Let’s face it; most code simply doesn’t age well.
Refactoring helps us limit how long we have to live with our mistakes, helps us improve code over time, and stop the deterioration.
Future blogs will explore some of the refactors that covered in this pivotal book. What are some of your experiences with refactoring?
One of the requirements for the MVP pattern is to keep everything out of the UI (View) that does not have to be in the View. Data mapping is one of those things that we would like to move out of the UI. I tend to think of it along these lines, “If I wanted to expose this same functionality in a different UI what do I not want to have to rewrite?”
If the View defines an interface exposing read write properties for all of the data needed, which it should, then the interface could be simply passed to the Presenter and the presenter handling the data mapping. The View does not have to get involved.
The Model should also define an interface exposing read write properties for all of the data that it has. The presenter deals with the Model and the View exclusively through these interfaces. This results in code similar to this:
private void MapData()
{
View.Field1 = Model.Field1;
View.Field2 = Model.Field2;
View.Field3 = Model.Field3;
View.Field4 = Model.Field4;
...
View.FieldN = Model.FieldN;
}
This is very straightforard. All of the nuances of how to display the mapped data is handled by the View as it should be. All of the details for how to get the data is handled by the model as it should be and the Presenter is left tieing the two together.
But this looks like tedious code. Tedious code is error prone code and code that we would like to avoid. If both interfaces,the one exposed for the View and the one exposed for the Model, require the same set of properties, then surely we can use reflection to handle writing this tedious code for us.
Consider this code:
public static void CopyTo(object sourceObject, object targetObject)
{
object value = new object();
object[] param = new object[0];
foreach (PropertyInfo propertyInfo in sourceObject.GetType().GetProperties())
{
MethodInfo getMethod = propertyInfo.GetGetMethod();
// Ensure that there is a get method for the source object.
System.Reflection.PropertyInfo targetPropertyInfo =
targetObject.GetType().GetProperty(propertyInfo.Name);
if ((getMethod != null) && (targetPropertyInfo != null))
{
// Ensure that there is a set method in the target object.
if (targetPropertyInfo.CanWrite )
{
value = getMethod.Invoke(sourceObject, param);
targetPropertyInfo.SetValue(targetObject,
ConvertValueType(value, propertyInfo.PropertyType),
null);
}
}
}
}
This function expects two objects to be passed in. We loop through the properties of the first object looking for matching properties in the second object. If the second object has a matching property and the second object’s property is not read only, we will get the value from the first objest’s property and set it to the second object’s property. The only thing that is required is that the any properties tha the two objects have in common must be of the same type.
Armed with such as CopyTo method, our MapData method from earlier can be reduced to:
public void MapData()
{
CopyTo(Model, View);
}
Regardless of how many properties are involved this one call will handle the data mapping. All properties that are in common will be copied. Properties in the Model that are not in the View will be ignored. Properties that are in the View that are not in the Model will be uninitialized.
Additionally, a generic update method might look similar to this:
public void Update()
{
Model = Model.GetCurrent();
CopyTo(View, Model);
Model.Save();
}
Here we make sure that we have the current model data. We then overwrite any common properties with the values currently in the View. Finally, we save the Model. This works regardless of the number of properties involved. Properties in the Model that are not in the View will not be affected. Properties in the View that are not in the Model will not be persisted.
The presenter makes no assumptions about how the Model gets its current data or what happens when the Save is called. The presenter makes no assumptions about how the View displays its data. The seperation between the UI, business logic, and data storage is maintained. Common, redundnat, potentially error prone data mapping logic is moved out of all three components into a reusable library component called by the presenter.
In an earlier post, we started with logic similar to this:
IDataReader dr;
try
{
dr = DAL.GetSomeData(filter);
while (dr.Read())
{
if ( dr["field"] != System.DBNull.Value)
{
DateTime convertedDate = Convert.ToDateTime(dr["field"].ToString());
}
if ( dr["nextField"] != System.DBNull.Value)
{
Double convertedDouble = Convert.ToDouble(dr["nextField"].ToString());
}
. . . .
catch (Exception ex)
{
// Handle exception
}
finally
{
dr = null;
}
}
And showed how to reduce the Complexity to something similar to this:
IDataReader dr;
try
{
dr = DAL.GetSomeData(filter);
while (dr.Read())
{
DateTime convertedDate = (DateTime) RetrieveDataFromDataReader (dr, typeof (DateTime), “field”);
Double convertedDouble = (Double) RetrieveDataFromDataReader (dr, typeof (Double), “nextField”);
. . . .
}
}
catch (Exception ex)
{
// Handle exception
}
finally
{
dr = null;
}
}
This is good in that it reduces the Complexity to be a constant instead of making it dependent on the number of fields being retrieved. We are also doing something nice worth pointing out. By using IDataReader instead of SQLDataReader or OracleDataReader, we have removed a direct tie to a particular database platform.
The problem comes in when business logic is dependent on the field names. This dependency in essence decentralizes the data mapping logic. Also, there is no compiler support for getting the field names right. If you get the name of the field wrong, you get a run time error instead of a compile time error. We prefer compile time errors. If the name of a field changes for any reason and this mapping logic is embedded with business logic, then there are multiple places that may need to be changed.
A better approach is to encapsulate this data mapping logic in a proxy object. Business logic is then written against this proxy object and can be oblivious to any data mapping changes. We bragged earlier that using an IDataReader we removed a direct dependency to SQLDataReader or OracleDataReader. By using a proxy object, we remove ourselves from the dependency that a database is even used.
If the proxy object exposes a strongly typed property for every field in the underlying data source, then the business logic can be implemented against them and never have to worry about data mapping logic or data type conversion logic. If the underlying data source changes, only the proxy object has to change. The business logic that was written against the proxy object stays the same.
With such a strong separation between the data access logic and business logic, the business logic won’t get lost in the “noise” of the data access logic. The mechanics of data type conversion and data mapping can be safely hidden away allowing the business logic to focus on business rules. This will make the business logic easier to follow, less likely to include bugs, and be more easily maintained.
In an earlier blog, we discussed the need to reduce complexity and some of the problems that complex code can cause. We even discussed a reliable quantifiable metric for evaluating complexity.
For review, we want to use the CodeMetrics add-in to Reflector to track Cyclomatic Complexity and target methods that have a complexity greater than 10.
One common source for elevated complexity is leaving artifacts from the data access in the business logic. We are probably all familiar with code that looks like this:
IDataReader dr;
try
{
dr = DAL.GetSomeData(filter);
while (dr.Read())
{
if ( dr["field"] != System.DBNull.Value)
{
DateTime convertedDate = Convert.ToDateTime(dr["field"].ToString());
}
if ( dr["nextField"] != System.DBNull.Value)
{
Double convertedDouble = Convert.ToDouble(dr["nextField"].ToString());
}
. . . .
catch (Exception ex)
{
// Handle exception
}
finally
{
dr = null;
}
}
Here we have a common scenario of reading data from a data reader and verifying that the columns are not null. If they are not null, we convert the field to a strongly typed variable and then proceeded to implement business logic with the strongly typed variables. As you can see from the above code fragment, we are dealing with two fields from the database and we already have a complexity of at least 6. There is a direct correlation between the number of fields being processed and the complexity of the method. We cannot have a linear relationship between complexity and the number of fields that we are referencing and expect to keep complexity below 10. A better approach is needed.
A better way might mean adding a function similar to this:
public object RetrieveDataFromDataReader(IDataReader dr, Type dataType, string whichField)
{
if (dr[whichField] == System.DBNull.Value)
{
return GetNullRepresentationValue(dataType);
}
else
{
return Convert.ChangeType(dr[whichField], dataType);
}
}
Armed with such a method, we can now rewrite our original method tobe something like:
IDataReader dr;
try
{
dr = DAL.GetSomeData(filter);
while (dr.Read())
{
DateTime convertedDate = (DateTime) RetrieveDataFromDataReader (dr, typeof (DateTime), “field”);
Double convertedDouble = (Double) RetrieveDataFromDataReader (dr, typeof (Double), “nextField”);
. . . .
}
}
catch (Exception ex)
{
// Handle exception
}
finally
{
dr = null;
}
}
This new version should have a constant complexity regardless of the number of fields introduced. This is a great improvement.
Now, there most likely is still business logic embedded in the data access logic. This is also bad, but it is not a problem made apparent through a strict review of code complexity metrics.
Next time, we will review why it is bad to merge the data access logic and business logic and review some easy ways to tease apart these interdependencies.
I recently worked on a project that failed. There is no other way to put it. The project failed! It’s a hard thing to admit.
There are many reasons why bad things happen to good projects. There are many reasons why despite our best intentions, not every project is successful. There is always plenty of blame to go around. The business market changed. The technology changed. We could not find the right resources. Critical resources left. The project lost funding. The business lost interest…
Many of these reasons can be boiled down to, IT didn’t know what the business needed and the business didn’t know what IT was doing. In other words requirements were not well documented.
From my recent experiences, I have come up with the following red flags to help you identify when your project might be in trouble
Red flags
· If you have never met your subject matter experts, your project might be in trouble
· If there are no subject matter experts, your project might be in trouble
· If new requirements are deemed critical but ignored for the sake of the schedule, your project might be in trouble
· If a user that you have never heard of claims to be a project sponsor, your project might be in trouble
· If the requirements change so much from one week to the next that you feel like it’s a different project, your project might be in trouble
· If new people join the team and its weeks before you even meet them, the project might be in trouble
· If you look around and there is no one to QA your code, your project might be in trouble
· If you spend more time deploying than you do coding, your project might be in trouble
· If the same project has failed over three times already, your project might in trouble
· If no one can succinctly state the business needs being solved, your project might be in trouble
· If you have already deployed once, but still second guessing the platform, your project might be in trouble
· If you can submit last month’s status report for this month and no one notices, your project might be in trouble
· If you can’t tell last month’s status report from this month’s, your project might be in trouble
· If you have worked on the project for over a month and still don’t know where the database is, your project might be in trouble
Guidelines:
· Write requirements down
· Don’t get bogged down in format or structure just content
· Get consensus that what you are working on is what needs working on
· Talk to as many users are you can. They rarely will agree with each other
· Deploy new functionality as often as you can so that the users don’t worry that you have forgotten about them
· Keep deployment schedules spread out enough so that you don’t get bogged down in deployment overhead
What red flags have you seen? What are some guidelines that you have seen work?