Avoiding transposed parameters without creating coupling

Question

I'm currently building a test application that manages parts for engineers, and I've hit a snag. I have a few different classes, including PartsModel and EngineerModel, and I want to update a list of parts that an engineer has, but I'm mindful of issues from either transposed parameters or from structuring the code in a way that unnecessarily couples to a particular class.

The two classes, with some relevant properties:

    public class PartModel
    {
        public int PartId { get; private set; }
        public string PartTitle { get; set; }
        public string PartDescription { get; set; }
        public int Quantity { get; set; }
        public int MinimumStock { get; set; }

        public void AddToStock (int quantityToAdd) {
            Quantity += quantityToAdd;
        }

        public void RemoveFromStock (int quantityToRemove) {
            Quantity -= quantityToRemove;
            CheckMinimumStock();
        }
    }

    public class EngineerModel
    {
        public int EngineerId { get; private set; }
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public List<PartModel> PartsInStock { get; set; } = Factory.CreatePartsList();
    }

As you can see, each engineer has a list of parts they have in stock via a List<PartModel>. I want to pass another list to this one so that I can update it respectively (incrementing or decrementing quantities, and then adding or removing parts to the list as necessary).

The first warning bell is that it takes two inputs of the same type, and is going to fill one from the other one (which isn't needed afterwards), so you're essentially modifying one input and destroying the other. To me, this presents a danger of the inputs getting transposed and the wrong list being either returned or updated (depending on whether it returns or just acts on the list). Because it removes items that have no quantity, it can't check the list length and just update the longer one, because there are possible cases where the engineer's list is shorter (maybe they're a new engineer, or maybe they just had a large shipment of parts sent when they were running low on stock). If it did just keep parts with quantity zero, then you're threatening scalability of both engineers and parts (not to mention any other objects that use the same operation).

So, put it as a method in the EngineerModel class and operate on PartsInStock, right? But what about when I want to use the same operation on other classes (e.g. if I have a list of parts associated to a work task)? Then I extract the method out to another class and... I'm passing the two lists as parameters in the method, so I'm back to where I was.

Am I being reasonable in not wanting to have two parameters of the same type, and how do I structure the code to deal with this, but without creating unnecessary coupling? If I'm not being reasonable, what am I overlooking?

Why not pass a single `List` with positive or negative Quantity? — David Browne - Microsoft, Jul 12 '20 at 14:50
@DavidBrowne-Microsoft I'm not sure I understand. That feels like the situation I describe in the penultimate paragraph - passing a single list to operate on the class property directly, but that means I can't extract that out to reuse the operation in other relevant places. I feel like I'm missing something. — Myles, Jul 12 '20 at 14:57
Where the method goes is a different issue. If you don't want it on `EngineerModel` You could make a custom PartModelCollection type, or make it an extension method targeting `List`. — David Browne - Microsoft, Jul 12 '20 at 15:01
That's the problem that I'm trying to solve, though. If I put it anywhere other than `EngineerModel`, I have to give it two lists (the list to be updated, and the list with the updates), which leads to my question: How do I get both lists there without accidentally swapping them over and returning the wrong list? — Myles, Jul 12 '20 at 15:24
Either option I suggested differentiates the parameters, enabling the calling code to execute a method with a single parameter. Eg `parts.AddParts( partsToAdd );` — David Browne - Microsoft, Jul 12 '20 at 16:28

Myles · Accepted Answer · 2020-07-20T13:08:15.297

Use an extension method

Thanks to @DavidBrowne-Microsoft for clarifying this. By defining an extension method for List<PartModel>, it only needs the one parameter - the list containing the updates (foreach below based on @Valentin's answer to this question).

public static class PartsHandler
{
    public static List<PartModel> UpdateStockQuantitiesWith(this List<PartModel> stockToBeUpdated, List<PartModel> stockUpdates) {
        foreach ( var part in stockUpdates )
        {
            var partToBeUpdated = stockToBeUpdated.FirstOrDefault(x => x.PartId == part.PartId);
            if ( partToBeUpdated != null )
            { partToBeUpdated.Quantity += part.Quantity; }
            else
            { stockToBeUpdated.Add(part); }
        }
        stockToBeUpdated.RemoveAll(x => x.Quantity <= 0);

        return stockToBeUpdated;
    }
}

Now any class that needs to implement this can simply call it in a method on the respective property. For example, in the EngineerModel class, it can operate on the PartsInStock property:

public void AddPartsToStock(List<PartModel> partsSent) {
    PartsInStock.UpdateStockQuantitiesWith(partsSent);
}

Better, but it's still possible to accidentally reverse it. When I write extensions like this, I like to include words like "from" or "to" in the method name for additional clarity. You might rename your method `UpdateFrom`. — StackOverthrow, Jul 12 '20 at 18:08
@StackOverthrow Thank you, I've renamed it to `UpdateStockQuantitiesWith` so that it's clear that it uses `stockUpdates` to update `stockToBeUpdated`. — Myles, Jun 25 '21 at 13:18

Avoiding transposed parameters without creating coupling

1 Answers1