3 Comments How code generation breaks YAGNI - 01/19/09

My post-title was getting too long, so I’ll just finish it here:

… and what you should do about it!

Actually, this is a bit of a follow up to post to my post about design principles. I felt I had something more to say about YAGNI in combination with software factories. Not very long ago, I talked about a software factory I layed my hands on.
Now, I’ve had the chance to mess around with it a little more.

Imagine starting a new project with a software factory. It generates a lot of code. Some of it usefull, some of it not very.
I’m asking two questions:
1) What code is actually usefull? => It depends on the case
2) What code is not usefull at all? => It depends on the case

According to YAGNI, we shouldn’t be writing code that we’re not going to use. But in this case, a lot of code we’re not going to use, has already been generated by the software factory. So the software factory breaks the YAGNI principle.
I don’t find it abnormal that the code generated by a SF breaks the YAGNI principle, but that doesn’t mean it’s OK to keep the code breaking YAGNI when using a SF!

Why does a SF break the principle?

A software factory, solves a common problem. But as we all know, every problem domain, has exceptions. A software factory will only be able to solve that what’s -generally- the same over all cases, thus the solution offered by the SF, will not be entirely correct in all cases.
Since we’re not starting from scratch with the codebase, the developer can’t avoid breaking the principle, since it’s the SF that breaks it. But as I said, that doesn’t mean it’s ok. While in cases we don’t use a SF, it’s the developer’s responsibility not to break the principle, now it’s the developer’s responsibility to clean up the mess the SF made.

Time to refactor the software factory?

If the SF is generating code that breaks YAGNI, doesn’t that mean it’s time to refactor? Well, yes and no. The SF is nothing more than an existing codebase, just like any other one. So it evolves with time, and like in every project, it also needs it’s refactoring love once in a while. We all know refactoring is a good thing.
During your refactoring-time, you could take into account what code to add, but most importantly since I’m talking about YAGNI here, what code to remove.
But that’s not an easy decision, since that code is usefull in one project, but not in the other one. The only code you can delete without thinking about it twice, is the code that’s just -never- used.
The code that deserves some serious brainstorming, headaches and analysis, is the code that is used in less than 80% of the cases.

The SF guys might be thinking: 80%??? Then you’re taking away 20% of value? And I’ll say, no, I’m not at all. I’m taking away the code, that is an added value for 20% of your projects, but that is an added misvalue (i had to give the kid a name) for the other 80% of the projects! I must say the developers that created this SF, thought about YAGNI carefully. If you only have about 20% that is YAGNI-code in code generated by a SF, I think we can say we’ve got ourselves a powerfull thingie here.

But still, you’ve got that overhead. That 20% of other code that’s only used in special cases.
You can’t delete it, still it bothers the hell out of you. And more important, it breaks the YAGNI principle! It makes your code base too big for nothing, and sometimes very confusing to work with.

Things I think should not be generated in the SF

- method overrides that only call your base implemenation: override them when you need them, typing “identifier override methodName” is not that hard, is it?
- tests that aren’t testing anything but prepare your mocks and variables: you could still have this partly handled by the SF, by creating a recipe for them

I’m sure I can add things, but these are the most obvious ones I’ve seen until now.

So what do we do about it?

Well, in my suggestion, I would just leave it in there… But not until the end of time!
You have to pay off your technical debt, and in this case it means you have to remove all YAGNI code! Plan some time to do this after you’ve finished a user story. If this seems to be too often, and you’re not removing hardly any code, you could choose to do this after each iteration. But don’t prolong it more than that, or you’ll end up not doing it at all, since it will take too much time to do it all in one go.

You’ll think, but what if I need that code in another user story, or an upcoming iteration? That’s where you’re breaking the principle. No excuses. After you’ve finished a user story or an interation, and you haven’t used the code, the chance you will be using it all, decreases a lot.
If you need it anyway in the end, just add it manually, it won’t kill you, I promise.

Important note!

If for some reason you need to regenerate your code (executing DSL’s for example), you should review all generated code for unused code again! And not after the iteration. I think in this case, it’s something you should do instantly. Do it at the same moment you regenerate your code. Consider it a part of regenerating your code. If you notice you’re regenerating a lot of code in different places while your change is small, then I would advice to take a look at your SF. It should offer you a way to regenerate parts of code, without affecting other ones.

How to identify YAGNI code?

Resharper already makes it easy to see YAGNI code (it will look gray) such as:
- unused overrides
- unused private methods
- unused parameters
- redundant code

But that won’t get you all the way through the cleanup. ReSharper only detects these redundancies locally (I’m hearing in a next version this option will be available solution wide), and it also gets harder with unused types. There are other tools that help you out with those problems.

FxCop is a great tool offered by Microsoft to perform static code analysis on compiled assemblies. On the other hand, -and I personally think this is the most powerfull code analysis tool on the market-, we’ve got NDepend. It analyzes your code, and let’s you examine dependencies, complexity, and  a bunch of other stuff using code metrics, graphs, other visual representations and -last but certainly not least-, CQL queries. NDepend already executes some CQL queries during the analysis, but afterwards you can even execute more specific CQL queries to analyze your codebase.

In our case, we’re looking for unused code. This CQL query is included in the analysis, so it’s veeeery easy! Just take a look at the results, and refactor them away.

For your information, here are the basic CQL queries NDepend uses for detection of dead code:

WARN IF Count > 0 IN SELECT TOP 10 TYPES WHERE
TypeCa == 0 AND     // Ca=0 -> No Afferent Coupling -> The type is not used in the context of this application.
!IsPublic AND       // Public types might be used by client applications of your assemblies.
!NameIs "Program"   // Generally, types named Program contain a Main() entry-point method and this condition avoid to consider such type as unused code.

WARN IF Count > 0 IN SELECT TOP 10 FIELDS WHERE
FieldCa == 0 AND  // Ca=0 -> No Afferent Coupling -> The field is not used in the context of this application.
!IsPublic AND     // Although not recommended, public fields might be used by client applications of your assemblies.
!IsLiteral AND    // The IL code never explicitely uses literal fields.
!IsEnumValue AND  // The IL code never explicitely uses enumeration value.
!NameIs "value__" // Field named 'value__' are relative to enumerations and the IL code never explicitely uses them.

WARN IF Count > 0 IN SELECT TOP 10 METHODS WHERE
MethodCa == 0 AND            // Ca=0 -> No Afferent Coupling -> The method is not used in the context of this application.
!IsPublic AND                // Public methods might be used by client applications of your assemblies.
!IsEntryPoint AND            // Main() method is not used by-design.
!IsExplicitInterfaceImpl AND // The IL code never explicitely calls explicit interface methods implementation.
!IsClassConstructor AND      // The IL code never explicitely calls class constructors.
!IsFinalizer                 // The IL code never explicitely calls finalizers.

If you haven’t looked at NDepend yet, I strongly advice you to do so!

Then, you’ll have a non-YAGNI codebase again.

3 Comments Fluent nHibernate: Classic mapping - 01/14/09

As I mentioned in my previous post, Fluent NHibernate offers two different ways to realize your mappings in C# code:

  • - adding a Map-class per entity and write out the mapping in C#
  • - use the Automapping feature in Fluent NHibernate and let the framework do all the work for you
  • In this post I’ll show you how you can write a basic mapping of an entity using a ClassMap.

A simple example

I’m starting out with a very simple example, with just one entity and a value object. I’ll be extending this tiny domain a bit, step by step (probably in another post).

fluentmodel

In my DB, I just have one table, Person. It contains all the address-data, but in my domain, I want to map that data into a seperate object, a value object. If you were using NHibernate, you could do just that using the “component”-tag in ugly XML.

Using classic mapping, you create a new class, PersonMap, which should derive from the generic class, ClassMap.

Here’s the code:

public class PersonMap : ClassMap<Person>
{
	public PersonMap()
	{
		Id(x => x.PersonId);
		Map(x => x.FirstName);
		Map(x => x.LastName);
		Map(x => x.PhoneNumber); 

		Component<Address>(x => x.Address, m =>
		{
			m.Map(x => x.Street);
			m.Map(x => x.Number);
			m.Map(x => x.ZipCode);
			m.Map(x => x.City);
			m.Map(x => x.Country);
		});
	}
}

If you want to specify the length of the FirstName property as in your database, or constraint the property to never allow NULL, you can do this as follows:

 Map(x => x.FirstName)
   .CanNotBeNull()
   .WithLengthOf(100);

I mentioned in my introduction post that FNH also gives you the ability to code by convention. It states that a developer should only specify the items that don’t fit within the default behaviour, thus to limit the configuration to a minimum. This makes your code easily maintanable since you don’t have configuration code in which you can get lost, you only have what you need.

Let me demonstrate that with an example. Check out how I mapped my Id property in the ClassMap. Notice I didn’t specify anything at all? Well, that’s because FNH assumes you’re using the identity generator. If you’re not, you can override this by specifying another generator type for your ID. Thus, you will only need to specify how you’ll be generating your ID’s if your not using identity.

Why? This avoids a lot of setup code when all your ID’s use this generator, don’t you think? Still, the flexibility remains. If you want to use another generator type, or explicitly state you are using the identity generator, you can still do so.

New way of testing

First of all, just a note. When I’m playing around with something new, just as now I’m playing around with FNH, I almost never write an application that actually does something. I’m just adding unittests to check if what i’m doing works.

FNH introduces a new way of testing your mappings. Remember how we used to create integration tests that add data to the database, retrieve it afterwards to check if it was successfully inserted? Remember how it was frustrating to notice in those tests that you messed up your NH mapping?

Actually, this was my way of testing if I did my mappings right. If any CRUD-operation failed, it was almost abvious I messed up my mapping somewhere. We have a test “CanAddPerson” which is also responsible to check if your mappings our correct. That’s not good, but there wasn’t another way…

Well, now you can separate the testing of your mappings, and your actual integration tests.

Here’s how you test your mappings:

[TestMethod]
public void CheckPersonMappingIsValid()
{
	new PersistenceSpecification<Person>(new SessionSource(new TestModel()))
		.CheckProperty(x => x.FirstName, "Laila")
		.CheckProperty(x => x.LastName, "Bougria")
		.CheckProperty(x => x.PhoneNumber, "0498123456")
		.CheckProperty(x => x.Address, new Address("My street", "34", "BE-2000", "Antwerp", "Belgium"))
		.VerifyTheMappings();
}

And here’s the integration test which only has the responsibility to check whether we can add Person’s to the datastore or not.

[TestMethod]
public void CanAddPerson()
{
	Person personToAdd = new Person()
							 {
								 Address = new Address("street", "15A", "BE-2100", "city", "country"),
								 FirstName = "Laila",
								 LastName = "Bougria",
								 PhoneNumber = "0497123456"
							 };

	Session.Save(personToAdd);
	Session.Flush();
	Session.Clear();

	// use session to try to load the person
	var fromDb = Session.Get<Person>(personToAdd.PersonId);

	// Test that the person was successfully inserted
	Assert.IsNotNull(fromDb);
	Assert.AreNotSame(personToAdd, fromDb);
	Assert.AreEqual(personToAdd.Address, fromDb.Address);
	Assert.AreEqual(personToAdd.FirstName, fromDb.FirstName);
	Assert.AreEqual(personToAdd.LastName, fromDb.LastName);
	Assert.AreEqual(personToAdd.PhoneNumber, fromDb.PhoneNumber);
}

I ran the tests, and they both pass, so that’s great!

A note about value objects and fluent nHibernate

In my Person example, I’m mapping the address-data with a value object. In the model you’ll see it implements IEquatable.
Of course it does, you’ll think, it’s a value object! I just want to warn you, that if you think “I’ll implement the interface later…” and you run the mapping test, it wil fail. The VerifyMappings method will be comparing the Address instance and will expect true if all the properties match. If you didn’t implement the interface yet, your test will fail.

|