Ayende @ Rahien

Refunds available at head office

Some code just make me queasy

And this one hits a lot of the high notes in that regard:

image

Three properties to say the same thing?!

Tags:

Posted By: Ayende Rahien

Published at

Originally posted at

Comments

Tengiz
05/14/2010 10:59 PM by
Tengiz

Dev A said "there only so many Genres and it'll be easy to query.

A found another gig and left. Second dev looked at it and said: 'The other guys design sucks! I will normalize it so it looks normalized and so i don't have to change anything when they rename punk to classical." He added GenreId and did not remove Genre so 'nothing breaks'. He got fired when no 'pop' items where sold in the third quarter 'cause no one could find any in the store. The subsequent consulting firm used an ORM that had a mapping convention setup by a departed employee to expect FK columns to be named as
<entityReference. They did not know that it could be changed. So they worked hard and added another column so that customers would be able to find 'pop' items. - That's how it went.

Demis Bellot
05/14/2010 11:28 PM by
Demis Bellot

@Tengiz

Sounds like a normal story on a multi-developer project, but isn't this pre-release software and yet the current maintainer never bothered to re-factor it?

I mean if the current maintainer can't be sure that he can modify existing software and be sure 'nothing breaks' then it sounds like you're dealing with an un-healthy project with a very dim future. IMHO any hindrance to change or re-factoring are one of the main causes that will lead to an un-maintainable code-base.

Surely this is one of the benefits of working in a statically-typed language?

In a code-first world where the db schema is binded to the C# model than a simple 'Find Usages' or a 'Remove field' and 'Re-compile all' to make sure nothing breaks will pick up any errors.

This is why I always try to code-first where-ever possible and have C# to talk to the outside world rather than the more cubersome alternative of mapping the outside world onto your C# models.

I'm only guessing this is what has happened here, otherwise leaving them all in doesn't make any sense.

dave-ilsw
05/15/2010 01:02 AM by
dave-ilsw

Or somebody decided that it was too expensive to have to look up the Genre name in the Genre table when displaying a list of Albums, so they decided to duplicate the Genre name in the Album table.

Simon
05/15/2010 02:56 AM by
Simon

Is this an IdeaBlade devforce project?

Dmitry
05/15/2010 03:53 AM by
Dmitry

It looks like an Entity Framework context badly configured.

There is no need to expose references if you enable lazy loading and prevent the designer to generate code. Also, there is no need to expose foreign keys when you can create stub objects.

Jarek Kowalski
05/15/2010 04:39 AM by
Jarek Kowalski

This looks like entity class generated with EFv4 designer usign default settings:

  • Genre - is a reference - most likely to a Genre class

  • GenreId - is a foreign key (new thing in EFv4)

  • GenreReference - special object which implements IRelatedEnd through which you can use to manipulate the reference in a generic way (to achieve explicit reference loading, etc.) - this concept has been around since first version of EF and is kept around mostly for back-compat reasons.

I know it does not look pretty, but EF provides automatic fixup to keep properties in sync - you can basically change one property and the other ones will be synchronized (or nulled out if synchronization is not possible because the object ha not been loaded).

You can argue whether the fixup (or in general any non-trivial behavior of proeprty setters) is the best thing, but it helps automate some common scenarios, such as data binding and serialization.

BTW. This is not the only way to write an object model in EF - you can totally write your classes as POCOs. Neither GenreId or GenreReference are required in this case.

jmorris
05/15/2010 06:56 AM by
jmorris

Looks like just another bit of shitty code that needs refactoring...nothing to see here move along.

Stephen
05/15/2010 02:54 PM by
Stephen

I'm sure you could just use the different entity generators, hell even the POCO one if you wanted, it would be more work, but compared to nhibernate, its still less work.

El Guapo
05/16/2010 05:21 AM by
El Guapo

This is nothing. You guys want to see something really scary? Try using EF on a table that has mutliple FKs to another table.

Comments have been closed on this topic.