SOLID-principle attempt, solid or not solid?

490 views Asked by At

In our layered architecture I am designing a BLL logic component called AppHandover and have written the basic high level code for this. I want it to follow the SOLID-principles and be loosly coupled, adopt separation of concern and be testable.

Here is what AppHandover should do

  • Check if User owns app. If not throw an error
  • remove history if possible (ie no more apps are assigned to user)
  • transfer the ownership to the next instance

Quesion is, am I on the right track and does the following sample seem SOLID?

public interface ITransferOwnership
{
    void TransferOwnership(string userId, string appId, TransferDirection transferDirection);
}
public interface IOwnershipVerification
{
    bool UserOwnsApp(string userId, int budgetId, string appId);
}
public interface IPreserveHistoryCheck
{
    bool ShouldDeleteTemporaryBudgetData(string userId, int budgetId);   
}
public interface IRemoveHistory
{
    void DeleteTemporaryBudgetData(string userId, int budgetId);
}

Handover process implementation

public class AppHandoverProcess : KonstruktDbContext, ITransferOwnership
{
    private IOwnershipVerification _ownerShipVerification;
    private IPreserveHistoryCheck _preserveHistory;
    private IRemoveHistory _removeHistory;
    private ITransferOwnerShip _transferOwnership;

    public AppHandoverProcess()
    {

    }
    public AppHandoverProcess(IOwnershipVerification ownerShipVerification, 
        IPreserveHistoryCheck preserveHistory, 
        IRemoveHistory removeHistory)
    {
        _ownerShipVerification = ownerShipVerification;
        _preserveHistory = preserveHistory;
        _removeHistory = removeHistory;
    }

    public void PerformAppHandover(string userId, string appId, int budgetId)
    {
        if (_ownerShipVerification.UserOwnsApp(userId,budgetId,appId)) {
            if (_preserveHistory.ShouldDeleteTemporaryBudgetData(userId, budgetId))
            {
                _removeHistory.DeleteTemporaryBudgetData(userId, budgetId);
            }

            //handover logic here..

            _transferOwnership.TransferOwnership(userId, appId, TransferDirection.Forward);
        }
        else
        {
            throw new Exception("AppHandover: User does not own app, data cannot be handed over");
        }
    }
}
2

There are 2 answers

1
Francesco Panina On BEST ANSWER

Concerning the code you outlined above I definitely think you're on the right track. I would push the design a little further and define TransferOwnership as an additional interface.

Following this approach your AppHandoverProcess is completely decoupled from it's client and the behaviour will be defined in the service configuration.

Enforcing an isolation for the TransferOwnership will allow you to easily UnitTest any object implementing the interface without the need to mock AppHandoverProcess dependency.

Also any AppHandoverProcess test should be trivial as the only thing you'll need to make sure is the your services are invoke or that the exception is thrown.

Hope this make sense,

Regards.

1
alltej On

I would make KonstruktDbContext as an injectable dependency. AppHandoverprocess should not inherit from it as it looks like it is a different responsibility.