This is a review of the S#arp Lite project, the version from Nov 4, 2011.

So far, we have gone over the general structure and the domain. Next on the list is going over the tasks project. No idea what this is. I would expect that to be some sort of long running, background tasks, but haven’t checked it yet.

Unfortunately, this isn’t the case. Tasks seems to be about another name for DAL. And to make matter worse, this is a DAL on top or a Repository on top of an OR/M.

And as if that wasn’t enough to put my teeth on edge, we got some really strange things going on there. Let us see how it goes.

Basically, a CudTask seems to be all about translating from the data model (I intentionally don’t call it domain model) to the view model. I spoke about the issues with repositories many times before, so I’ll suffice with saying that this is still wasteful and serve no real purpose and be done with it.

This TransferFormValuesTo() is a strange beast, and it took me a while to figure out what is going on here. let us look in the parent class to figure out what is going on there.

Let me count the things that are wrong here.

First, we have this IsTransient() method, why do we have that? All we need to do is just to call SaveOrUpdate and it will do it for us. Then the rest of the method sunk in.

The way this system works, you are going to have two instances of every entity that you load (not really true, by the way, because you have leakage for references, which must cause some really interesting bugs). One instance is the one that is managed by NHibernate, dealing with lazy loading, change management, etc. The second is the value that isn’t managed by NHibernate. I assume that this is an instance that you get when you bind the entity view the action parameters.

NHibernate contains explicit support for handling that (session.Merge), and that support is there for bad applications. You shouldn’t be doing things this way. Extend the model binder so it would load the entity from NHibernate and bind to that instance directly. You wouldn’t have to worry about all of this code, it would just work.

For that matter, the same goes for validation as well, you can push that into NHibernate as a listener. So all of this code just goes away, poof!

And then there is the Delete method:

I mean, is there a rule that says “developers should discard error information as soon as possible, because it is useless.” I mean, the next step is to see C# code littered with things like:

catch(Exception e)
{
   delete e; // early release for the memory held by the exception
}

The one good thing that I can say about the CudTasks is that at least they are explicit about not handling reads, and that reads seems to be handled properly so far (but I haven’t looked at the actual code yet).

Rob Conery has been talking about REST lately, and I think he perpetuate a common misconception. In particular, in the post I referenced, he is asking about ideas for URLs for doing things like logging in, working with productions and episodes, etc.

The problem with that is that this has very little to do with REST. Now, I’ll be the first that will tell you that discussions about architectural purity bore me, and I really like the concept of nice URLs. But nice URLs are totally different from REST.

These slides do a really good work of describing what REST is and how to work with it.

It wasn’t until I actually was called to do a code review on an application written by Rob Eisenberg that I really got it. That application was a pretty simple UI (well, the UI logic was simple, the UI itself was pretty complex, but that was mostly because of the visualizations). The interesting thing is that most of the UI was completely driven by the response from the server.

What I mean by that is that when you loaded an entity, it would load the appropriate view, and use information like this:

<link method="DELETE" title="Cancel" rel="rels/cancelOrder" href="/orders/1234"/>

<link method="GET" title="Shipping Details" rel="rels/viewShipping" href="/orders/1234/shipping"/>

To generate much of the actual behavior on the client side.

The client was fairly stable, but modifying the server meant that you could get a lot more from the system.

Human readable and hackable urls are nice, sure. But they have very little to do with REST.

RavenDB is in heavy use inside MSNBC.COM, running some of their most interesting assets.

On Wed, Mar 7, 2012 9:00 AM - 10:30 AM PST, we will host the development team for doing that in our RavenDB Webinar to learn what is it like to run RavenDB on the most popular U.S. news site.

In their own words: "we’re using RavenDB, loving it, and plan to use it much more".

In this webinar, we will talk with members of the development team responsible for that, learn how they:

• deal with RavenDB in production
• manage geo distribution with RavenDB
• rapidly make changes in both development and production.

Please register to the webinar, we have a limited amount of participants, so make sure to register in advance.

And bring your own questions, we will open this up for the audience to ask their own questions.

This is a review of the S#arp Lite project, the version from Nov 4, 2011.

In my previous post, I looked at the general structure, but not much more. In this one, we are going to focus on the Domain project.

We start with the actual domain:

I have only few comments about this sort of model:

• This is a pure CRUD model, which is good, since it is simple and easy to understand, but one does wonder where the actual business logic of the system is. It might be that there isn’t any (we are talking about a sample app, after all).
• The few methods that are there are also about data (in this case, aggregation, and Order.GetTotal() will trigger a lazy loaded query when called, which might be a surprise to the caller.
• Probably the worst point of this object model is that it is highly connected, which encourages people to try to walk the object graphs where they should issue a separate query instead.

Next, let us look at the queries. We have seen one example where NHibernate low level API was hidden behind an interface, but that was explicitly called out as rare. So how does this get handled on a regular basis?

Hm… I have some issues here with regards to the naming. I don’t like the “Find” vs. “Query” naming. I would use WhereXyz to add a filter and SelectXyz to add a transformation. It would read better when writing Linq queries, but that is about it for the domain.

One thing that I haven’t touched so far is the entities base class:

And its parent:

I strongly support the notion of ComparableObject, this is recommended when you use NHibernate. But what is it about GetTypeSpecificSignatureProperties? What it actually does is select all the properties that has the [DomainSignature] attribute. But what would you want something like that?

Looking at the code, the Customer.FirstName and Customer.LastName have this attribute, looking at the code, I really can’t understand what went on here. This seems to be selected specifically to create hard to understand and debug bugs.

Why do I say that? The ComparableObject uses properties marked with [DomainSignature] for the GetHashCode() calculation. What this means is that if you change the customer name you change its hash code value. This hash code value is used for, among other things, finding the entity in the unit of work, so changing the customer name can cause NHibernate to loose track of it and behave in some really strange ways.

This is also violating one of the core principals of entities:

A thing with distinct and independent existence.

In other words, an entity doesn’t exists because of the particular values that are there for the first and last names. If those change, the customer doesn’t change. It is the same as saying that by changing the shirt I wear, I becomes a completely different person.

Domain Signature is something that I am completely opposed, not only for the implementation problems, but because it has no meaning when you start to consider what an entity is.

Next, we are going to explore tasks…

This is a review of the S#arp Lite project, the version from Nov 4, 2011.

I was asked to review this project a long time ago, but I never got around to it. I had some time and I decided that I might take a look and see how it goes. I don’t like the S#arp Arch project, because it seems too complex and heavy weight for the purpose.

The project comes with a sample application, which is good, because it is easy to see how the framework is intended to be used. Unfortunately, it is yet another online store example, I am getting heartily sick of that. On the other hand, it is a fairly simple model and easy to understand, so I grok why this keeps getting chosen.

Review Rule, I look at the code. If I wanted to deal with documentation, I would write some for our products. I am doing this because I find it fun to look at other people’s code. So skip any comments about “if you read the docs…”.

We start from the project structure:

I am not sure if I like it, I don’t know if I agree that all of those splits are needed, but this is well within reasonable limits, so I am willing to let it slide on the grounds that this is personal taste more than anything else. Looking at the dependencies, we see:

The Init project contains two files, which are responsible for… well, starting up, it seems. Again, I don’t see any reason why this would be a separate project, but that is about it so far.

Next in line is the NHibernateProvider project, in this case, we have the following:

So far, I am cautiously optimistic. All of the files / folders marked with red are actually all about setting NHibernate up, not about hiding it. But then we get to the read me file, which reads in part:

This folder contains any concrete, NHibernate-specific query classes.
There should only be classes in here for any respective query *interfaces* found in
/MyStore.Domain/Queries/

This folder will usually be empty except for very exceptive cases.

This is… interesting. Can’t say whatever I agree or not yet. Looking at the QueryForProductOrderSummaries, we see:

Note the comment, there are better ways to do it, but we demonstrate an ugly way, and how to nicely encapsulate it.

That is enough for now, I think, next post, I’ll touch on the actual model…