0
\$\begingroup\$

There is the following method that should get record from the database, apply new data, and if at least one value has been changed, update database and send event

We don't use EntityFramework, it uses CosmosDb SDK in the repository.

I'm not sure how is the proper way to implement hasChanges.

public async Task<Unit> Handle(ChangeLiftStateCommand request, CancellationToken cancellationToken)
{
    Lift lift = await _repository.Get(request.Number, cancellationToken)
        ?? throw new NotFoundException(request.Number);

    bool hasChanges = false;
    if (Enum.TryParse(request.HealthState, ignoreCase: true, out HealthState healthState))
    {
        if (lift.State.HealthState != healthState)
        {
            lift.State.HealthState = healthState;
            hasChanges = true;
        }
    }

    if (lift.State.GenericState != request.GenericState)
    {
        lift.State.GenericState = request.GenericState;
        hasChanges = true;
    }

    if (request.CabinPosition.HasValue)
    {
        Cabin cabin = lift.Cabins.FirstOrDefault(c => c.Type == CabinType.Main);
        if (cabin is null)
        {
            lift.Cabins.Add(new Cabin
            {
                Deck = Deck.Lower,
                Position = request.CabinPosition.Value
            });
            hasChanges = true;
        }
        else
        {
            if (cabin.Position != request.CabinPosition.Value)
            {
                cabin.Position = request.CabinPosition.Value;
                hasChanges = true;
            }
        }
    }

    if (hasChanges)
    {
        await _repository.Update(lift);

        await _eventProducer.Send(new LiftChangedEvent
        {
            Number = request.Number,
            CabinPosition = request.CabinPosition,
            HealthState = request.HealthState,
            GenericState = request.GenericState
        });
    }

    return Unit.Value;
}

Could you please advice how it can be refactored? Are there any pattern for hasChanges implementation? Thank you in advance

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

You can move HealthState and Cabin portions into separate methods as each one of them serve a single purpose for a single property. So, moving them into their own methods would give more freedom and extensibility such as adding more requirements, some other future changes .etc. Plus, it would be reusable in the current scope, and you can make it reusable into other scopes by exposing them or using extension methods.

For me, I feel extension methods would be the way to go like this :

public static ChangeLiftStateCommandExtensions
{
    public static bool TryUpdateHealthState(this ChangeLiftStateCommand request, Lift lift)
    {
        if (Enum.TryParse(request.HealthState, ignoreCase: true, out HealthState healthState))
        {
            if (lift.State.HealthState != healthState)
            {
                lift.State.HealthState = healthState;
                 return true;
            }
        }
        
        return false;
    }

    public static bool TryUpdateCabin(this ChangeLiftStateCommand request, Lift lift)
    {
        Cabin cabin = lift.Cabins.FirstOrDefault(c => c.Type == CabinType.Main); 
        
        if (request.CabinPosition.HasValue)
        {
            if (cabin is null)
            {
                lift.Cabins.Add(new Cabin
                {
                    Deck = Deck.Lower,
                    Position = request.CabinPosition.Value
                });
                
                return true;
            }
            else
            {
                if (cabin.Position != request.CabinPosition.Value)
                {
                    cabin.Position = request.CabinPosition.Value;
                    
                    return true;
                }
            }
        }
        
        return false;
    }   
}

Then, your actual method would changed to this :

public async Task<Unit> Handle(ChangeLiftStateCommand request, CancellationToken cancellationToken)
{
    Lift lift = await _repository.Get(request.Number, cancellationToken)
        ?? throw new NotFoundException(request.Number);

    
    bool isHealthStateUpdated = request.TryUpdateHealthState(lift);

    bool isCabinUpdated = request.TryUpdateCabin(lift);
    
    if (isHealthStateUpdated || isCabinUpdated)
    {
        await _repository.Update(lift);

        await _eventProducer.Send(new LiftChangedEvent
        {
            Number = request.Number,
            CabinPosition = request.CabinPosition,
            HealthState = request.HealthState,
            GenericState = request.GenericState
        });
    }

    return Unit.Value;
}

you can also override the Equals on Lift, so you can make a copy of the object as source copy, and work with one copy, and when you're done, compare both versions.

Something like this :

Lift lift = await _repository.Get(request.Number, cancellationToken); 
Lift liftSource = lift;

/// your work 


if(!lift.Equals(liftSource))
{
    // object has changes
}
\$\endgroup\$
4
  • \$\begingroup\$ Thanks. Do you think I should add an extension for GenericState too? \$\endgroup\$ Commented Oct 23, 2021 at 23:20
  • \$\begingroup\$ Lift contains man properties and those properties contain other inner properties. Very huge structure. Is it possible to override Equals in some elegant way? \$\endgroup\$ Commented Oct 23, 2021 at 23:22
  • \$\begingroup\$ use extension method on ChangeLiftStateCommand if you see it is not going to be used else where ! but if you think that you would do the same procedure else where then you probably need to do the methods else where. (you could add it to the Lift directly, or implement a Manager class and add all methods into it that suppose to handle Lift procedures. \$\endgroup\$
    – iSR5
    Commented Oct 24, 2021 at 1:28
  • \$\begingroup\$ for the equals, if the class is huge, then don't override the equals, instead, you can add the methods directly to it, or use extension methods on Lift directly. \$\endgroup\$
    – iSR5
    Commented Oct 24, 2021 at 1:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.