I’m wondering if there is a better way to do this or if I’m doing some anti-pattern here. Like I say in the subject, I’m trying to share repository calls and object mapping logic between Unit of Work methods.
For example see my GetEnvelopeBody() method being used more than once.
public class EnvelopeService
{
private EnvelopeRepository envelopeRepository;
private RecipientRepository recipientRepository;
private RecipientTabRepository recipientTabRepository;
public Envelope GetEnvelope(int id)
{
using (var uow = UnitOfWorkFactory.Create())
{
envelopeRepository = new EnvelopeRepository(uow);
recipientRepository = new RecipientRepository(uow);
recipientTabRepository = new RecipientTabRepository(uow);
// I'm extracting the repository calls and mapping
// code into this GetEnvelopeBody() so other methods
// that have their own Unit of Work can use it.
// This is to avoid multiple Unit of Work sessions?
// from colliding.
var envelope = GetEnvelopeBody(id);
uow.SaveChanges();
return envelope;
}
}
private Envelope GetEnvelopeBody(int id)
{
// repository calls and object mapping code goes here
}
public void SaveEnvelope(Envelope envelope)
{
using (var uow = UnitOfWorkFactory.Create())
{
envelopeRepository = new EnvelopeRepository(uow);
recipientRepository = new RecipientRepository(uow);
recipientTabRepository = new RecipientTabRepository(uow);
SaveEnvelopeBody(envelope);
uow.SaveChanges();
}
}
private void SaveEnvelopeBody(Envelope envelope)
{
CleanEnvelope(envelope);
// repository calls and object mapping code goes here
}
private void CleanEnvelope(Envelope newEnvelope)
{
var oldEnvelope = this.GetEnvelopeBody(newEnvelope.Id);
// Do other fancy stuff ...
}
}
1
The major problem I see is that your EnvelopeService class is not threadsafe.
Imagine two threads using a single instance of EnvelopeService. Thread 1 calls GetEnvelope which creates a UnitOfWork. At the same time Thread 2 calls SaveEnvelope which creates a second UnitOfWork. Whether those two operations are carried out using the appropriate UnitOfWork is not guaranteed.
This is because you’re effectively storing your UnitOfWork at the class level (in your 3 repository instances) and then using it in subsequent helper method calls. This is shared state and is unsafe.
A safer approach would be to remove the shared state:
public class EnvelopeService
{
public Envelope GetEnvelope(int id)
{
using (var uow = UnitOfWorkFactory.Create())
{
var envelopeRepository = new EnvelopeRepository(uow);
var recipientRepository = new RecipientRepository(uow);
var recipientTabRepository = new RecipientTabRepository(uow);
var envelope = GetEnvelopeBody(id, envelopeRepository, recipientRepository, recipientTabRepository);
uow.SaveChanges();
return envelope;
}
}
private Envelope GetEnvelopeBody(
int id,
EnvelopeRepository envelopeRepository,
RecipientRepository recipientRepository,
RecipientTabRepository recipientTabRepository)
{
// repository calls and object mapping code goes here
}
public void SaveEnvelope(Envelope envelope)
{
using (var uow = UnitOfWorkFactory.Create())
{
var envelopeRepository = new EnvelopeRepository(uow);
var recipientRepository = new RecipientRepository(uow);
var recipientTabRepository = new RecipientTabRepository(uow);
SaveEnvelopeBody(envelope, envelopeRepository, recipientRepository, recipientTabRepository);
uow.SaveChanges();
}
}
private void SaveEnvelopeBody(
Envelope envelope,
EnvelopeRepository envelopeRepository,
RecipientRepository recipientRepository,
RecipientTabRepository recipientTabRepository)
{
CleanEnvelope(envelope, envelopeRepository, recipientRepository, recipientTabRepository);
// repository calls and object mapping code goes here
}
private void CleanEnvelope(
Envelope newEnvelope,
EnvelopeRepository envelopeRepository,
RecipientRepository recipientRepository,
RecipientTabRepository recipientTabRepository)
{
var oldEnvelope = this.GetEnvelopeBody(newEnvelope.Id, envelopeRepository, recipientRepository, recipientTabRepository);
// Do other fancy stuff ...
}
}
Notice that the repositories holding your your UnitOfWork are now passed into your helper methods as dependencies.
Whether you should be newing your repositories is another potential point of concern. It might make testing the EnvelopeService class difficult. Consider using a Factory to get your repository instances instead. Then again, that might be overkill. It really depends on your situation.
1