Skip to main content
Code Review

Return to Question

Tweeted twitter.com/StackCodeReview/status/1259271983847690241
Improved example
Source Link
FDaniels
FDaniels
  • 41
  • 3

Edit: There was a question in regards to how it would normally be done in this application. Here was the evolution of the usage statement

First Implementation

if (storedIndividual.Status == (int)IndividualStatus.InProgress &&
    (individual.StatusModel.Id == (int)IndividualStatus.Submitted || 
        individual.StatusModel.Id == (int)IndividualStatus.Disqualified))
        
{
    SetFinishedIndividualFormValues(ref individual);
    if (individual.StatusModel.Id == (int)IndividualStatus.Submitted)
    {
        SendSubmittedFileEmail(individual);
    }
}
else if (individual.StatusModel.Id != (int)IndividualStatus.Disqualified)
{
    SendStatusChange(individual);
}

Revised Implementation

private void HandleStatusChanges(Models.Database.Individual storedIndividual, ref Models.Domain.IndividualDm individual)
{
    if (storedIndividual.Status == (int)IndividualStatus.InProgress &&
        (individual.Status == IndividualStatus.Submitted || 
            individual.Status == IndividualStatus.Disqualified))
    {
        SetFinishedIndividualFormValues(ref individual);
        if (individual.Status == IndividualStatus.Submitted)
        {
            SendSubmittedFileEmail(individual);
        }
    }
    else if (individual.Status != IndividualStatus.Disqualified)
    {
        SendStatusChange(individual);
    }
}

Edit: There was a question in regards to how it would normally be done in this application. Here was the evolution of the usage statement

First Implementation

if (storedIndividual.Status == (int)IndividualStatus.InProgress &&
    (individual.StatusModel.Id == (int)IndividualStatus.Submitted || 
        individual.StatusModel.Id == (int)IndividualStatus.Disqualified))
        
{
    SetFinishedIndividualFormValues(ref individual);
    if (individual.StatusModel.Id == (int)IndividualStatus.Submitted)
    {
        SendSubmittedFileEmail(individual);
    }
}
else if (individual.StatusModel.Id != (int)IndividualStatus.Disqualified)
{
    SendStatusChange(individual);
}

Revised Implementation

private void HandleStatusChanges(Models.Database.Individual storedIndividual, ref Models.Domain.IndividualDm individual)
{
    if (storedIndividual.Status == (int)IndividualStatus.InProgress &&
        (individual.Status == IndividualStatus.Submitted || 
            individual.Status == IndividualStatus.Disqualified))
    {
        SetFinishedIndividualFormValues(ref individual);
        if (individual.Status == IndividualStatus.Submitted)
        {
            SendSubmittedFileEmail(individual);
        }
    }
    else if (individual.Status != IndividualStatus.Disqualified)
    {
        SendStatusChange(individual);
    }
}
Source Link
FDaniels
FDaniels
  • 41
  • 3

Overloading == for better readability

I was working on a project recently that had a good amount of business logic associated with status codes. To better understand the flow of logic in the domain layer, I created an enum representation of the statuses and overloaded the equality operator in the status domain model to check against the enum.

Is this good practice, or does this an unnecessary abstraction? Also seeing if I should implement the GetHashCode() and Equal() methods or perhaps just make them just return

() => throw new InvalidOperationException();

Implementation

public class StatusDm
{

    public StatusDm() { }
    public StatusDm(Models.Database.Status status)
    {
        Id = status.Id;
        Name = status.Name;
    }
    public int Id { get; set; }
    public string Name { get; set; }


    public void DatabaseTransfer(ref Models.Database.Status status)
    {
        status.Name = Name;
    }
    public static bool operator ==(StatusDm domainStatus, Models.Constants.StatusEnum status)
    {
        return domainStatus?.Id == (int)status;
    }

    public static bool operator !=(StatusDm domainStatus, Models.Constants.StatusEnum status)
    {
        return !(domainStatus?.Id == (int)status);
    }
}

Usage

if(individual.StatusModel == StatusEnum.Submitted || individual.StatusModel == StatusEnum.Disqualified)
{
    ...
}

Appreciate any feedback or critic. Thanks!