The O in ORM

This code is not object-oriented enough. Objects are not oriented to objects; instead, objects are oriented to database detail, read: keys.

// Consumer code
var fields = Event.GetEventFields(da, actual.Event, language, language);

// Implementing code
public static IEnumerable<Dto.NotificationDto.Template.Response.EventFieldDto>
GetEventFields(IDomainAccess da, EventEnum @event, string lang, string fallbackLang)
{
    var list = 
        from e in da.Query<Event>()
        join f in da.Query<EventField>()
        on e.Enum equals f.Event.Enum

        where e.Enum == @event

        select new Dto.NotificationDto.Template.Response.EventFieldDto
        {
            FieldName = f.Field.TextXlat.Localize(lang, fallbackLang),
            FieldTag  = f.Field.Mnemonic
        };
}

When using a decent ORM like NHibernate, keys shouldn't leak as much as possible on the application layer, as keys could be composite. Joining and filtering with composite keys is a PITA, so just use object when joining and filtering.

Linq and NHibernate can facilitate that. It's only less capable ORM like Entity Framework that insist on accessing stuff through keys.

As much as possible, make objects communicate to objects.

To wit, here's how the ORM queries could be more object-oriented, oriented to object, instead of oriented to database. Foreign keys and primary keys are removed from the Linq query.

// Consumer code:
var fields = Event.GetEventFields(da, da.JustKey<Event>(actual.Event), language, language);


// Implementing code:
public static IEnumerable<Dto.NotificationDto.Template.Response.EventFieldDto>
            GetEventFields(IDomainAccess da, Event eventNeeded, string lang, string fallbackLang)
{

    var list = 
        from e in da.Query<Event>()
        join f in da.Query<EventField>()
        on e equals f.Event // notice that we can use the object themselves on joins. NHibernate is smart enough to know what is the primary key(s) of the object.

        where e == eventNeeded // same here. We don't need to use the primary key

        select new Dto.NotificationDto.Template.Response.EventFieldDto
        {
            FieldName = f.Field.TextXlat.Localize(lang, fallbackLang),
            FieldTag  = f.Field.Mnemonic
        };
    }

When using ORM, just forget the details of what is the primary key is, the primary key's property name may change or the primary could grow two composite key(two or more column keys), but the object itself stays the same.

So even if the name of the primary key (e.g., Id, Code, Enum) is changed to something else or primary key is changed to composite the suggested object-oriented Linq above would still work, future-proof.


Another thing to note, since there is a collection of EventField in Event itself, we can eliminate the join in code by using the EventField collection instead, and then flatten the Event's EventField collection by using SelectMany.


The join-using code above could be shortened to:

// Consumer code:
var fields = Event.GetEventFields(da, da.JustKey<Event>(actual.Event), language, language);


// Implementing code:
public static IEnumerable<Dto.NotificationDto.Template.Response.EventFieldDto>
            GetEventFields(IDomainAccess da, Event eventNeeded, string lang, string fallbackLang)
{
    var list = 
        from eventField in da.Query<Event>().SelectMany(e => e.Fields) 
                      
        where eventField.Event == eventNeeded

        select new Dto.NotificationDto.Template.Response.EventFieldDto
        {
            FieldName = eventField.Field.TextXlat.Localize(lang, fallbackLang),
            FieldTag  = eventField.Field.Mnemonic
        };

    return list.ToList();
}

Lastly, since the object can protect its children by enforcing protected access modifier, accesing the child from the aggregate directly is perfectly fine, it's still DDD as long things are accessed via aggregate's root.

// Consumer code:
var fields = Event.GetEventFields(da, da.JustKey<Event>(actual.Event), language, language);


// Implementing code:
public static IEnumerable<Dto.NotificationDto.Template.Response.EventFieldDto>
            GetEventFields(IDomainAccess da, Event eventNeeded, string lang, string fallbackLang)
{
    var list = 
        from eventField in da.Query<EventField>()

        where eventField.Event == eventNeeded

        select new Dto.NotificationDto.Template.Response.EventFieldDto
        {
            FieldName = eventField.Field.TextXlat.Localize(lang, fallbackLang),
            FieldTag  = eventField.Field.Mnemonic
        };

    return list.ToList();
}

Another approach is to get the aggregate root, and then project the collection, this way creating static method can be avoided.

// Consumer code:
Event eventNeeded = da.Get<Key>(actual.Event);
var fields = eventNeeded.GetEventFields(lang, fallbackLang); 


// Implementing code:
public IEnumerable<Dto.NotificationDto.Template.Response.EventFieldDto>
 GetEventFields(string lang, string fallbackLang)
{
    var list = 
        from eventField in this.Fields
        // where eventField.Event == eventNeeded // this will not be necessary anymore
        select new Dto.NotificationDto.Template.Response.EventFieldDto
        {
            FieldName = eventField.Field.TextXlat.Localize(lang, fallbackLang),
            FieldTag  = eventField.Field.Mnemonic
        };

    return list.ToList();
}  

However, for performance-conscious developers or users, the drawback of not using static method when accessing the aggregate's children is it will incur two queries to database. A query for getting a single row from parent, and another query for getting the children of the parent.


To improve the performance of the code above while giving the illusion of accessing the aggregate's intance method instead directly, use extension method. Thanks C#! :)

The code below will just issue one query to database, on event_field table only, event table will not be queried.

From a consumer:

var eventNeeded = da.JustKey<Event>(actual.Event);
var fields = eventNeeded.GetEventFields(da, language, language); // GetEventFields looks like an instance method and not an static method.

The instance method illusion helper:

public static class EventPerformanceHelper
{
    public static IEnumerable<Dto.NotificationDto.Template.Response.EventFieldDto>
    GetEventFields(this Event eventNeeded, IDomainAccess da, string lang, string fallbackLang)
    {
        var list =
            from eventField in da.Query<EventField>()

            where eventField.Event == eventNeeded

            select new Dto.NotificationDto.Template.Response.EventFieldDto
            {
                FieldName = eventField.Field.TextXlat.Localize(lang, fallbackLang),
                FieldTag  = eventField.Field.Mnemonic
            };


        return list.ToList();
    }
}

Read more about using DDD while not sacrificing performance: http://www.ienablemuch.com/2013/12/pragmatic-ddd.html

Highfalutin code #4. With great power, comes great responsibility.

No matter how powerful our programming language is..

foreach (var item in 
    documentFields.SelectMany(
        documentField => documentField.Options, 
        (documentField, option) => new { documentField, option }
    )
)
{
   item.option.DocumentField = item.documentField;
}

..we still have a great responsibility to make our code readable and maintainable:

foreach (var documentField in documentFields)
    foreach (var option in documentField.Options)
    {
        option.DocumentField = documentField;
    }

SelectMany's intent is to flatten *ALL* the option collection from all fields, and then assign each option item in collection to their parent field.

Though the objective of the first code is to make the program's intent obvious, its verbosity blurs the intent of the code.

In fact, if we want to make it appear we are flattening the collection, we can also do it as follows:

foreach (var documentField in documentFields) foreach (var option in documentField.Options)
{
    option.DocumentField = documentField;
}

If we look at the two for loops as projecting a table with two columns, the projected options will now look like flattened rows.

We just have to tilt our perspective sometimes :)


Disclaimer: I won't use the last code.

Sometimes longer code is ok

This code even it's longer..

.Select(permission => (Common.Security.PermissionEnum) System.Enum.Parse(typeof(Common.Security.PermissionEnum), permission.Value));

..is superior to this short code:

.Select(permission => (Common.Security.PermissionEnum) int.Parse(permission.Value));

The first code, even if the enum is updated to long data type, will still work.


How often do we upgrade the enum to long though?


Happy Coding!