Minimal DTOs - Enum Example

This page supplements a point I make on the main "Minimal DTOs" page.

Example: Database - Web App

Let's look at an enum that represents the status of an order. Let's say that an order can be pending, fulfilled, or cancelled - and any other status is invalid. We'll need to establish what that looks like at every level of the stack. I will omit as many details as I can, but I want to build up a truly working example, so that my point doesn't get lost in any handwaving.

Since there is so much of this code, I'll be highlighting the parts that are particularly relevant. Starting with the database, which enforces the validity of the status: 


CREATE TABLE [Order] (
	Id INT IDENTITY(1,1),
	ProductId INT,
	Quantity INT,
	PricePerUnit DECIMAL,
	[Status] VARCHAR(20)
)

ALTER TABLE [Order] ADD CONSTRAINT
	CK_Order_Status CHECK ([Status] IN ('Pending', 'Fulfilled', 'Cancelled'))

Now some code to read and write to the database: 


public enum OrderStatus
{
	Pending, Fulfilled, Cancelled
}

public class Order
{
	public int Id { get; set; }
	public int ProductId { get; set; }
	public int Quantity { get; set; }
	public decimal PricePerUnit { get; set; }
	public OrderStatus Status { get; set; }
}

public class OrderDao
{
	public Order GetOrder(int orderId)
	{
		using (var conn = new SqlConnection(_connstr))
		{
			conn.Open();

			var query = "SELECT * FROM [Order] WHERE Id = @orderId";
			using (var command = new SqlCommand(query, conn))
			{
				command.Parameters.AddWithValue("@orderId", orderId);

				using (var reader = command.ExecuteQuery())
				{
					if (reader.Read())
					{
						return new Order
						{
							Id = (int)reader["Id"],
							ProductId = (int)reader["ProductId"],
							Quantity = (int)reader["Quantity"],
							PricePerUnit = (decimal)reader["PricePerUnit"],
							Status = (OrderStatus)Enum.Parse(
								enumType: typeof(OrderStatus),
								value: (string)reader["OrderStatus"]),
						};
					}
					else
					{
						return null;
					}
				}
			}
		}
	}

	public void SetOrderStatus(Order order)
	{
		using (var conn = new SqlConnection(_connstr))
		{
			conn.Open();

			var query = "UPDATE [Order] SET [Status] = @Status WHERE Id = @Id";
			using (var command = new SqlCommand(query, conn))
			{
				command.Parameters.AddWithValue("@Id", order.Id);
				command.Parameters.AddWithValue("@Status", order.Status.ToString());
				command.ExecuteNonQuery();
			}
		}
	}
}

Great! Let's write an API for this: 


[ApiController]
public class OrderController : ControllerBase
{
	[HttpGet]
	[Route("order/{id}")]
	public IActionResult GetOrder(int id)
	{
		var order = _dao.GetOrderById(id);
		if (order is null)
		{
			return NotFound();
		}
		else
		{
			return Ok(order);
		}
	}

	[HttpPost]
	[Route("order/{id}")]
	public IActionResult UpdateOrder(Order order)
	{
		_dao.UpdateOrderStatus(order.Id, order.Status);
		return Ok();
	}
}

Finally, a frontend to consume that API. It will use JSON: 


<!DOCTYPE html>
<html lang="en">
<head>
	<meta charset="utf-8">
	<title>Demo</title>
</head>
<body>
	<input id="id-input" type="text" />
	<button id="get-button">Get</button>
	<span id="product-span"></span>
	<span id="quantity-span"></span>
	<span id="price-span"></span>
	<select id="status-select">
		<option value="Pending">Pending</option>
		<option value="Fulfilled">Fulfilled</option>
		<option value="Cancelled">Cancelled</option>
	</select>
	<button id="update-button">Update</button>
</body>
<script>
	const API_URL = "https://api.mycompany.com"; // or whatever

	function el (id) {
		return document.getElementById(id);
	}

	function getOrder () {
		fetch(`${API_URL}/order/${el('id-input').value}`)
		.then(response => response.json())
		.then(displayOrder);
	}

	function displayOrder (order) {
		el('product-span').innerText = order.productId;
		el('quantity-span').innerText = order.quantity;
		el('price-span').innerText = order.pricePerUnit;
		el('status-select').value = order.status;
	}

	function updateOrder () {
		fetch(`${API_URL}/order/${idInput.value}`, {
			method: 'POST',
			headers: {
				'Content-Type': 'application/json;charset=UTF-8',
			},
			body: JSON.stringify({
				id: el('id-input').value,
				productId: el('product-span').innerText,
				quantity: el('quantity-span').innerText,
				pricePerUnit: el('price-span').innerText,
				status: el('status-select').value,
			}),
		})
		.then(response => console.log(response));
	}

	el('get-button').addEventListener("click", getOrder);
	el('update-button').addEventListener("click", updateOrder);
</script>
</html>

We are almost done (assuming CORS and some other irrelevant details have been properly addressed). But unless we do one more thing, the frontend will fail to read OrderStatus from the API.

That last step is in the API configuration. In Startup.cs, or Program.cs, or SiteBuilder.cs, or wherever the heck this version of this API framework stashes its configuration code, we need to tell the JSON serializer to treat enums as strings. For the framework and version I am currently using, the incantation looks like this: 

builder.Services.AddControllers()
	.AddJsonOptions(options =>
	{
		// This is using System.Text.Json.Serialization;
		var converter = new JsonStringEnumConverter()
		options.JsonSerializerOptions.Converters.Add(converter);
	});

Wow, that was a long walk. Thanks for sticking with me. Take a breath before we dive back in.

Let's ask, what would this look like if Status were a string?

And what would we lose?

Well, suppose the frontend developer made a mistake when writing that dropdown, and spelled "Cancelled" as "Canceled". If the API were using an enum, this would likely result in an HTTP 400 (Bad Request) response (assuming our middleware binding is doing its job well). If the API uses a string, that "Canceled" would make it all the way to the database before it caused an error with the check constraint, ultimately causing a less specific HTTP 500 (Server Error) response.

In theory we could address this by doing more validation work in the API, returning 400 if appropriate before calling the database, but why bother? The frontend developer will still have to check the API logs for details, whether they are getting a 400 or 500. The error message will be decipherable, if not downright readable, either way. And then the dropdown will be fixed, and this error will never happen again. Until we decide to add a fourth status (more on that later)...

Another thing we lose is compiler support for any other places we refer to Status in the rest of the API. Presumably it includes some checks of the Status; perhaps some business logic to ensure that Cancelled orders can't become Fulfilled. More concretely, we would go from this: 


public void FulfillOrder(int orderId)
{
	var order = _dao.GetOrderById(orderId);
	if (order.Status == OrderStatus.Cancelled)
	{
		throw new InvalidOperationException("Cannot fulfill a cancelled order");
	}
}

To this: 


public void FulfillOrder(int orderId)
{
	var order = _dao.GetOrderById(orderId);
	if (order.Status == "Cancelled")
	{
		throw new InvalidOperationException("Cannot fulfill a cancelled order");
	}
}

Here, the "Canceled" spelling bug would not cause an immediate error, but it would allow invalid behavior. I posit that the likelihood of that bug being introduced, missed in unit testing, and missed in QA testing is so low, that we would do better to focus on preventing our more common logical bugs.

As for the readability of the enum code compared to the string code... while readability is subjective, I don't believe that removing the prefix and adding quotes has any significant effect on readability, in either direction.

"But Ben, shouldn't we take any opportunity we can to reduce the probability of bugs, no matter how small?"

On the one hand, no we should not. If we become so bug-paranoid that we turn ourselves into the space shuttle autopilot development team, having each line triple-certified before anything is ever deployed, we will move at such a painfully glacial pace that we will never be able to release another feature before the market opportunity for that feature has vanished. This is hyperbole, of course; I just want you to consider that it is possible to go too far in the quest for perfect code.

On the other hand, it is actually quite easy for us to get that compiler support back. Here's how: 


public static class OrderStatus
{
	public const string Pending = "Pending";
	public const string Fulfilled = "Fulfilled";
	public const string Cancelled = "Cancelled";
}

public void FulfillOrder(int orderId)
{
	var order = _dao.GetOrderById(orderId);
	if (order.Status == OrderStatus.Cancelled)
	{
		throw new InvalidOperationException("Cannot fulfill a cancelled order");
	}
}

Voilà. We have our cake, and are eating it too.

Adding a value

There is only one constant in software, and that is change. Even our fixed set of possible statuses is not as fixed as we might like. What if we need to implement pre-orders? We would probably want a fourth possible status: "Scheduled". What would that take?

Whether OrderStatus is a string or an enum, we will need to update the database check constraint. We will also need to update the dropdown on the UI. If OrderStatus is just a bare string, now we are done.

If OrderStatus is an enum, or a string constant in a static class, we must also go update that. If OrderStatus is a string constant, now we are done.

If OrderStatus is an enum, we must also go update all of the consumers everywhere that might include that enum, via DLL or Nuget package. Failure to do so will result in very sneaky bugs "sometime later," as Orders with the Scheduled status gradually reach more and more systems. This is a pain.

What if we want to prevent the need to update the UI? That's possible, by creating a service or endpoint with the sole responsibility of listing the allowable Order.Status values. You might call this the "Reference Data" approach, and it will work whether the status is a string or an enum. I haven't used that approach here, for the obvious reason that it was too complex to include in my already overly-detailed example.

I personally prefer not to use the "Reference Data" approach, but I do want to acknowledge that it exists, and it is valid. The reason for my preference is the usual: I don't think that the complexity justifies itself, in most cases. To really explore the tradeoffs would take us too far off topic today, so look for that sometime in the future.

Parsing

Suppose we receive order data from some third party, outside of our control. Let's call them Acme. Acme uses the same set of statuses we do, but they send us abbreviations instead of full status names. So we need to apply some translation.

That translation might live in the service that handles messages from Acme, and if Acme sends us JSON, it might look like this: 


public Order ParseOrder(string acmeOrderJson)
{
	var parsedJson = JsonDocument.Parse(acmeOrderJson).RootElement;

	return new Order
	{
		Id = parsedJson.GetProperty("id").GetInt32(),
		ProductId = parsedJson.GetProperty("productId").GetInt32(),
		Quantity = parsedJson.GetProperty("quantity").GetInt32(),
		PricePerUnit = parsedJson.GetProperty("price").GetDecimal(),
		Status = ParseStatus(parsedJson.GetProperty("status").GetString()),
	};
}

private string ParseOrderType(string acmeStatusAbbreviation)
{
	switch (acmeStatusAbbreviation)
	{
		case "S": return "Scheduled";
		case "P": return "Pending";
		case "F": return "Fulfilled";
		case "C": return "Cancelled";
		default: throw new ArgumentOutOfRangeException(
			$"Cannot parse status '{acmeStatusAbbreviation}' from Acme JSON.");
	}
}

What would change if we used string constants instead of "raw" strings here? Approximately nothing; it would be a cosmetic change.

What would change if we used an enum instead of strings here? Again, almost nothing. The cosmetics would be identical to the string constants.

But it would be very tempting to put the enum parsing in a central location; perhaps in the DTO itself, or perhaps in a class dedicated to parsing and stringifying that particular enum (and I believe this temptation is stronger with enums than with strings). Centralizing that logic would mean that it needs to be able to handle input from not just Acme, but other parts of the system. We might want to make it case-insensitive, or even let it handle the integer values of the enum (0 = Pending, 1 = Cancelled, 2 = Fulfilled, etc).

That would have the unfortunate side effect of hiding the behavior of our own system from ourselves (and future maintainers of our code). If we universally accept "s" as meaning "Scheduled", how will our maintainers know that only Acme sends us anything like that (and doesn't even send us that exactly)? What is to stop them from sending "s" around in other parts of the system?

I have saved this argument for last on this page because it is the most tangential, and the weakest. I have given it its own section ("Setter parsing") on the main page, where it is more relevant. In short, I agree more with the criticism of Postel's Law than I do with the law itself.