Well structured code, but with issues

Jun 29, 2010 at 10:17 AM
I browsed the source code and the well structured code gives a nice first impression. Sadly, there are issues I don't like. It could very well be my preferences, or the way the current tool set works. * When I navigated to ExpenseComponent, the first thing I saw was an empty class, because everything was hidden in a region. Please use regions to hide noise, not real business code. * Then I agree with the contributor who noticed that CreateExpence should sit on the Expence object itself. I understand the reasoning why it isn't so, but I don't see the conflict: These methods aren't meant to run client side, so it doesn't matter the serialization kills them. * I like the way data persistence is separated from the business entities, except for this constant in the Expence class: EntitySetName = "Expenses". Now, the business entities aren't separated from the persistence anymore. * The business entities are also used as WCF data contracts. I consider that, if not bad practice, an unfortunate practice. The WCF boundary is there for a reason, to separate our clients from our internal business implementation. In this case, the boundary is no longer there, so any change to our business entities will most probably break our clients.
Coordinator
Jul 3, 2010 at 3:25 AM

Hi Teyde,

As there are many ways to design a system, some design choices made in the sample were merely personal and others could be related to the tools and technology.

The Business Entities are used to transfer data only and do not (should not) contain persistence methods. The reasons for this is:

  • Visual Studio allows us to re-used reference assemblies when Adding a Service Reference. This will indirectly make us deploy persistence logic to the client side if we were not careful. This of course will only happen when we are the developer of both service consumer and provider. It doesn't happen when we are developing on other consuming platforms.
  • Having persistence methods in Entities mean that Entities are tied to Data Layer components which will cause performance issues when managing transaction objects and serialization. ADO.NET EF may have solved that but I have retained the similar structure because I never know what data technologies I may use next.

The EntitySetName is ugly. I don't like it either but ADO.NET Entity Framework needs it. Perhaps there is another way which I have not discovered but at the moment, it is required. However, don't let the name "Expenses" mislead you into thinking that it is the table name because it is actually the Entity Set Name that was defined in the model. Therefore, if you change it to "MyExpenses" in both the entity code and model, it will still work against the Expenses table. It has no relationship with the physical table.

Finally, I took the approach of making the entities as data contracts for simplicity. In larger systems which I built, I have message contracts. But regardless of what contracts we used, if there is a business requirement that causes a breaking change in our data structure, and if we don't follow proper contract versioning rules, it will always break client code. No matter how many boundaries or layers we create, it will still break. Having said this, I think I will illustrate Message Contracts in the next release of the sample :)

Thank You for your feedback.

Best Regards,
Serena

P.S. I will remove the region in the business component :)

Jul 3, 2010 at 12:37 PM
Edited Jul 3, 2010 at 12:38 PM

A suggestion for the Entity Set Name - I don't know if there are any other internal use of the EntitySetName, but as far as I can read from the code in the example, the only place the EntitySetName property is accessed is in the <Entity>DataAccess classes.

Would it be possible to put the constant somewhere in this (DataAccess-)class instead of in the entity? That way it is in the Data Layer and the Business layer is kept clean?

Coordinator
Aug 2, 2010 at 7:35 AM

In the next revision,

  1. MessageContracts will be used to pass data to/from Expense Workflow Service
  2. Entity Set Name is moved to Data Access Components

Thanks.