> Mike Valenty

Refactoring C# Style

| Comments

Take a look a this function.

1
2
3
4
5
6
7
8
9
10

[UnitOfWork]
public virtual void Upload(DocumentDto dto)
{
    var entity = new Document().Merge(dto);

    using (var stream = new MemoryStream(dto.Data))
    {
        repository.Save(entity, stream);
    }
}

After doing some performance profiling, it quickly popped up as the top offender. Why? Because it’s holding a database transaction open while saving a file. In order to fix it, we have to ditch our UnitOfWork aspect and implement a finer-grained transaction. Basically what needs to happen is saving the entity and saving the file need to be separate operations so we can commit the transaction as soon as the entity is saved. And since saving the file could fail, we might have to clean up an orphaned file entity.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53

public virtual void Upload(DocumentDto dto)
{
    var entity = new Document().Merge(dto);

    SaveEntity(entity);

    try
    {
        SaveFile(entity, dto.Data);
    }
    catch
    {
        TryDeleteEntity(entity);
        throw;
    }
}

private void SaveEntity(Document entity)
{
    unitOfWork.Start();
    try
    {
        repository.Save(entity);
        unitOfWork.Commit();
    }
    catch
    {
        unitOfWork.Rollback();
        throw;
    }
}

private void SaveFile(Document entity, byte[] data)
{
    using (var stream = new MemoryStream(data))
    {
        repository.Save(entity, stream);
    }
}

private void TryDeleteEntity(Document entity)
{
    unitOfWork.Start();
    try
    {
        repository.Delete(entity);
        unitOfWork.Commit();
    }
    catch
    {
        unitOfWork.Rollback();
    }
}

That wasn’t too bad except that the number of lines of code exploded and we have a few private methods in our service that only deal with plumbing. It would be nice to push them into the framework. Since C# is awesome, we can use a combination of delegates and extension methods to do that.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59

public virtual void Upload(DocumentDto dto)
{
    var entity = new Document().Merge(dto);

    unitOfWork.Execute(() => repository.Save(entity));

    try
    {
        repository.SaveFile(entity, dto.Data);
    }
    catch
    {
        unitOfWork.TryExecute(() => repository.Delete(entity));
        throw;
    }
}

public static class DocumentRepositoryExtensions
{
    public static void SaveFile(this IDocumentRepository repository, Document document, byte[] data)
    {
        using (var stream = new MemoryStream(data))
        {
            repository.SaveFile(document, stream);
        }
    }
}

public static class UnitOfWorkExtensions
{
    public static void Execute(this IUnitOfWork unitOfWork, Action action)
    {
        unitOfWork.Start();
        try
        {
            action.Invoke();
            unitOfWork.Commit();
        }
        catch
        {
            unitOfWork.Rollback();
            throw;
        }
    }

    public static void TryExecute(this IUnitOfWork unitOfWork, Action action)
    {
        unitOfWork.Start();
        try
        {
            action.Invoke();
            unitOfWork.Commit();
        }
        catch
        {
            unitOfWork.Rollback();
        }
    }
}

Now we can move the extension methods into the framework and out of our service. In a less awesome language we could define these convenience methods on the IUnitOfWork interface and implement them in an abstract base class, but inheritance is evil and it’s a tradeoff we don’t have to make in C#.

Comments