Overloading == for better readability

Question

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!

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);
 }
}

• Mast: Interesting. I'm not sure it's a good idea to even consider overloading ==, but it's definitely interesting.
• Mast: Unfortunately your question seems to attract downvotes. This is probably due to the usage being a very small example, one that isn't doing much. However, in this case, I wouldn't know what else it should be showing to indicate how it's used. Do you perhaps have a project in which you've used this implementation? That usually works better on this site.
• FDaniels: @Mast I've update the question to showcase the changes in implementation. Hopefully it makes it clearer what I am trying to ask.
• Mast: Absolutely, welcome to Code Review.
• iSR5: Could you post both models Models.Database.Individual and Models.Domain.IndividualDm ?

Answer 1

It is difficult to judge without full code, but while keeping things mutable (please use a more descriptive names, a future you will thank you):

public class StatusModel
{
 public IndividualStatus Status { get; set; }
 public int StatusId { get => (int)Status; set => Status = (IndividualStatus)value; } 
 public bool InProgressStatus => Status == IndividualStatus.InProgress;
 public bool SubmittedStatus => Status == IndividualStatus.Submitted;
 public bool DisqualifiedStatus => Status == IndividualStatus.Disqualified;
 public bool DisqualifiedOrSubmittedStatus => DisqualifiedStatus || SubmittedStatus;
 public bool JustFinished(StatusModel stored) =>
 stored.InProgressStatus && DisqualifiedOrSubmittedStatus;
 public bool JustSubmitted(StatusModel stored) =>
 JustFinished(stored) && SubmittedStatus;
 public bool StillInProgress(StatusModel stored) =>
 !JustFinished(stored) && !DisqualifiedStatus;
}

As a next step you could also define an implicit conversion operators from/to int/enum types and make this class immutable.

public class StatusModel
{
 public static implicit operator StatusModel(IndividualStatus status) => new StatusModel(status);
 public static implicit operator IndividualStatus(StatusModel model) => model.Status;
 public static implicit operator StatusModel(int id) => (IndividualStatus)id;
 public static implicit operator int(StatusModel model) => (int)model.Status;
 StatusModel(IndividualStatus status) => Status = status;
 public IndividualStatus Status { get; set; }
 public bool InProgressStatus => Status == IndividualStatus.InProgress;
 public bool SubmittedStatus => Status == IndividualStatus.Submitted;
 public bool DisqualifiedStatus => Status == IndividualStatus.Disqualified;
 public bool DisqualifiedOrSubmittedStatus => DisqualifiedStatus || SubmittedStatus;
 public bool JustFinished(StatusModel stored) =>
 stored.InProgressStatus && DisqualifiedOrSubmittedStatus;
 public bool JustSubmitted(StatusModel stored) =>
 JustFinished(stored) && SubmittedStatus;
 public bool StillInProgress(StatusModel stored) =>
 !JustFinished(stored) && !DisqualifiedStatus;
}

It is a bunch of code (C# is a ridiculously verbose language), but it would be very easy to have more logic here, adding anything status related would be a very cheap operation.

private void HandleStatusChanges(
 Models.Database.Individual storedIndividual, 
 ref Models.Domain.IndividualDm individual)
{
 if (individual.Status.JustFinished(storedIndividual.Status))
 SetFinishedIndividualFormValues(ref individual);
 if (individual.Status.JustSubmitted(storedIndividual.Status))
 SendSubmittedFileEmail(individual);
 if (individual.Status.StillInProgress(storedIndividual.Status))
 SendStatusChange(individual);
}

I would also define and associate custom JsonConverter and TypeConverter with StatusModel, so it will look like a status integer ID for asp.net (see an example here).

P.S. Generally speaking, data entity types should never be visible in the business core: https://blog.cleancoder.com/uncle-bob/2016/01/04/ALittleArchitecture.html

Answer 2

Avoid overloading the == operator (and other operators) as this is a tricky business, and best avoided - so I wouldn't go down that route.

---

OK next the code.

Enums and Integer representations

If you can alter the Individual and IndividualDm classes.

Add something that returns the integer value as the Enum...

Simple version: Models.Database.Individual

public int Status {get; set;} // Assume this is the integer status value already implemented.
public IndividualStatus StatusType => (IndividualStatus)Status;

Simple version: Models.Domain.IndividualDm

public StatusModel Status {get;set;}
public IndividualStatus StatusType => (IndividualStatus)Status.Id;

If don't have the control (or any change the Status int may not exist as an Enum), then an Extension method or a simple conversion factory, would do. Happy to expand on this, if you want.

---

Design-Patterns

Your business logic is asking a bunch of questions, that I feel should be better represented in your domain models, and logical flow (e.g. Services)

For me, there is a lot of missing business logic information, and so this is just a earlier opinion, and based on a lot of assumptions.

Basic Rules / Model Questions

1. The storedIndividual.StatusType must be InProgress to be allowed to continue? (This is an assumption, as the code doesn't actually say this).
2. Has the Status changed?
3. Act on the change, with different processes.
4. Store the change / send an event of the change

Services type logic that runs the code...

var storedIndividual = _individualStore.GetById(request.Id);
// This is just a simple inversion of InProgress to make it a bit more flexible, see implementation in class
if (storedIndividual.IsClosed()) {
 return; // Or throw, or some specific error code
}
// Assumption, no action is need if the status hasn't changed.
var hasStatusChanged = storedIndividual.HasStatusChanged(individual.StatusType );
if (!hasStatusChanged) {
 return; // Or throw, or some specific error code
}
if (individual.StatusType == IndividualStatus.Submitted) {
 // SetFinishedIndividualFormValues
 // SendSubmittedFileEmail
 return;
}
if (individual.StatusType == IndividualStatus.Disqualified) {
 // SetFinishedIndividualFormValues
 return;
}
// SendStatusChange
// NB. It feels natural to me to call storedIndividual.SetStatus(individual.StatusType), here - but it really depends.

namespace Models.Database{
 public class Individual {
 ...
 // Assumption, Individual represents a record case, such as a mortgage application (?)
 // This could be expand further if there are other status that could allow it to be open.
 public bool IsClosed => this.StatusType!= IndividualStatus.InProgress; 
 public bool HasStatusChanged(IndividualStatus newStatus) {
 return this.StatusType != newStatus;
 }
 } 

• iSR5: I think the storedIndividual.Status stores the old status while the individual.StatusModel.Id stores the new status, and if that's true, then your example would return different results. since the OP logic is tighten to InProgress status.
• mrdnk: Well the Individual class should also implement the StatusType and use the Status.Id - though this depends on the implementation. But that was a good spot, will update in second
• mrdnk: Changed to use StatusType, thanks for comments from @iSR5. Note, I made an assumption about the Logic being tightened to InProgress, this may not be the case. In which case, we can re-evaluate.