The reverse of Carlsberg slogan applied to code
Monday, 30. June 2008, 19:08:53
I sometimes get very annoyed with code I stumble upon. It happens more often these days, when I'm doing for one of my clients a thorough code review, cleanup, low level refactoring and so on.
Today was a special day with this respect. I found something that made me mad, in the most Monthy Pythonesque way.
I found a piece of code similar to this:
For the people less familiar with the marvelous Microsoft universe, let me explain that we are using Entity Framework, which is a ORM for .NET, developed by Microsoft. So SomeEntity actually corresponds to a table in the database. The entity instances are managed in a special space called a context. As for the constructor, it has this strange form because it initializes properties of the class with some values.
Now, when any decent developer looks at this code, it becomes immediately apparent that the variable entity is a local variable which is not used again. So, it's useless. Right?
Right, but with one catch. It so happens that whenever you set a property based on a relationship between two entities, as in the example, the entity is also saved in the context. Yes, you understood it correctly. It's like doing something like:
and the dialog gets displayed.
The worst thing is that we didn't actually do anything to provoke something like this. This is how it works out of the box.
Disclaimer: it's true that we are using the beta 3 version of the Entity Framework. However, I am not very convinced that the final version will improve on this point...
Today was a special day with this respect. I found something that made me mad, in the most Monthy Pythonesque way.
I found a piece of code similar to this:
for(int i = 0; i < 100; ++i)
{
SomeEntity entity =
new SomeEntity{SomeOtherEntity = someValue};
}
For the people less familiar with the marvelous Microsoft universe, let me explain that we are using Entity Framework, which is a ORM for .NET, developed by Microsoft. So SomeEntity actually corresponds to a table in the database. The entity instances are managed in a special space called a context. As for the constructor, it has this strange form because it initializes properties of the class with some values.
Now, when any decent developer looks at this code, it becomes immediately apparent that the variable entity is a local variable which is not used again. So, it's useless. Right?
Right, but with one catch. It so happens that whenever you set a property based on a relationship between two entities, as in the example, the entity is also saved in the context. Yes, you understood it correctly. It's like doing something like:
Dialog dlg = new Dialog(); dlg.BackgroundColor = Color.White;
and the dialog gets displayed.
The worst thing is that we didn't actually do anything to provoke something like this. This is how it works out of the box.
Disclaimer: it's true that we are using the beta 3 version of the Entity Framework. However, I am not very convinced that the final version will improve on this point...
