Dynamic types in strongly typed languages - always wrong?
Tags: strong, weak, codedesign
Published: 2019-10-18 20:45:10

Recently I had a discussion with some colleagues about an endpoint I had created. I was asked by the product team to add the functionality to export some data in a specific format. We had endpoints that already did similar things and they all worked something like this:

 - Post some data to an endpoint
 - Write a row in a "file" table and remember the ID.
 - Raise a message with the data to a queue to start generating the document
 - Return the file ID to the front end so it can poll and API for the state of the file (and download when ready)

There were several of these endpoints already. The API didn't use the payload for anything, it simply put it on a queue but all these endpoints were the same code copied and pasted over and over. The only difference: the DTO classes for the payload they accepted.

Seeing this I decided I had to do something! I made a new endpoint that would become the standard export endpoint going forward.

![Standards XKCD](https://imgs.xkcd.com/comics/standards.png)

I was using C# so forgive me if you're unfamiliar, I will try and make it easy enough to follow. Here is the endpoint I wrote:

    [Route("/export/{type}/{documentName}/{format}")]
    public Guid ExportDocument([FromBody]JObject payload,
        string type, string documentName, string format)
    {
        var fileId = _fileRepo.NewFile(documentName, format);
        var message = new 
        {
            DocumentName = documentName,
            Format = format,
            Payload = payload,
            FileId: fileId
        };
        
        var queue = GetQueueFromType(type);
        queue.PublishMessage(ToJson(message));

        return fileId;
    }

<sup>(Not the real code, but close enough to get the idea)</sup>

In that example, the `JObject` is a C# class for representing some arbitrary JSON object, and that comes from the body of the HTTP request.

My thinking was: the next time someone needed to export a file in a different format they could use this endpoint and the only thing they would need to do is add their queue URL to mapping file (a simple JSON file). They could subscribe their new worker to the queue and messages would start making their way there.

I was fairly pleased with the design but it proved controversial when it came to making the pull request. The sticking point: the `JObject`. 

Colleagues said things like:

> This is C# not JavaScript, we should be have strongly typed objects. JObject is a code smell.

or 

>What if someone posts a payload that's missing data the worker needs to process the message? You should validate it on the API.

or 

>The API should serve as the documentation for what is a valid request. How will people know what to post if there isn't a DTO?

or

>Why are you trying to avoid creating new endpoints? We're not charged per endpoint!

I argued and still maintain this is the correct design for several reasons:

 - No code duplication. You could argue that duplication could be minimal for the solution with multiple endpoints if they all called back into a generic method but I still feel like the multiple endpoints themselves are a form of duplication; it's just more obviously duplicated when the endpoints are various versions of the same method copied and pasted, as was the case here.
 - No DTOs required. Who wants yet more useless classes in an already large codebase? I'm not fond of DTOs in C# anyway - I dislike the fact that the seem to find themselves spreading out everywhere unless they are kept on a short leash and I particularly dislike that they have to have public get and set methods for every property.
 - Reduced coupling. If the worker that's creating the documents changes and requires a new format for the payload the API doesn't need to change. The frontend needs to pass the new format and the worker needs to be able to handle the new format but the API will just pass along the payload as before.

I also just don't think a strongly typed endpoint has much of an advantage. It's not going to catch a type mismatch at compile time, it's a web API, the JavaScript can send anything. It just causes a runtime error which is exactly the same as the runtime error the worker would experience if the payload it received was incorrect. Perhaps it acts as a sort of documentation, but I feel like you could achieve similar documentation just by writing a comment above the endpoint saying: _Check the worker to find out what shape of payload is expected for each document type._

I managed to get the pull request in, but I feel like everyone else would still prefer if I got rid of it and went back to the old method. What do you think? Was it really a bad design? Are there disadvantages I've not mentioned? Would you have done something different? I feel like I just can't see the other side of the debate here so please help me understand in the comments.