2

I have a collection of Shipment objects that I am iterating through to update another system. A property on a Shipment is a Stockroom. The target system has a unique ID defining a Stockroom record. If I have a collection of Shipments and some of them relate to a Stockroom for which I've not yet recorded a key cross reference record to store the unique ID, then I need to perform a GET on that system's Stockroom resource for the first time. After doing so, I will save a key cross reference record but I also need to 1) inform the rest of the objects in the Shipments collection which use that Stockroom what its unique ID is, and 2) not attempt to perform additional GETs and key cross reference saves for the same Stockroom.

Due to questions of the data hierarchy, here is the full list of properties and the constructor for the Shipment object.

public string PopReceiptNumber { get; set; }
public int ReceiptLineNumber { get; set; }
public decimal QuantityShipped { get; set; }
public GPItem Item { get; set; }
public GPStockroom Stockroom { get; set; }

public GPShipment(DataRow row)
{
    PopReceiptNumber = row.Field<string>("POPRCTNM");
    ReceiptLineNumber = Int32.Parse(row.Field<string>("RCPTLNNM"));
    QuantityShipped = row.Field<decimal>("QTYSHPPD");
    Item = new GPItem(row);
    Stockroom = new GPStockroom(row);
}
public class GPStockroom
{
    public string Name { get; set; }
    public string Sys_ID { get; set; } // unique ID in target system
}

// Here is a snippet of code where pushing each Shipment happens
foreach (Shipment shipment in shipments)
{
    shipment.Integrate();
}

The Integrate() method on the Shipment class contains the code that will perform a GET from the target system if the Sys_ID of the Shipment's Stockroom is empty. I am wondering what is a good way to identify the rest of the objects in the collection of Shipments which have the same Stockroom Name value and update their Sys_ID. I see there are answers to this problem on StackOverflow suggesting using LINQ but then also see strong recommendations that this is not a best practice. The recommendation for clarity is to use a foreach loop. I am asking this question because I'm already in a foreach on the collection.

  • Is there a good reason why your Shipment's class Stockroom property is a string and not a ...Stockroom reference? This way, there will only be one reference for a specific Sys_ID, which will have the same name and, when initialized from the shipment.Integrate() method, its Sys_ID property will be updated once and all other Shipment instances will be accessing the same Stockroom reference, which will have been already initialized. This way, you will not have to update anything thereafter. If, however, you must have a string Stockroom property, see my answer. – Vector Zita Sep 23 at 22:54
  • That is a mistake, it should be a typed as a Stockroom. However, I don't follow your logic thereafter. Why are you thinking all Shipments will access the same Stockroom reference? – CodenameCain Sep 23 at 22:59
  • By the way, judging by your question, you are assuming that you will have multiple instances of Stockroom, with the same Name and Sys_ID... Usually, this is undesirable. Is this what you really want? – Vector Zita Sep 23 at 23:00
  • When Shipment instances are created, which Stockroom reference is passed to them? Is it always a new one? This way you have multiple Stockroom instances referring to the exact same real-world item, and they all have identical properties. Is there no way around this problem first? For example holding a repository (e.g. a Dictionary<string, Stockroom>) and checking within the Integrate() method if the required Sys_ID already corresponds to a created Stockroom, so that this can be immediately returned instead of a re-instantiation of an identical instance for that Stockroom? – Vector Zita Sep 23 at 23:03
  • Yes. I may be processing 5 Shipment transactions where 3 received quantity into Stockroom1, and two received quantity into Stockroom 2. Stockroom has one other custom object as a property called Item. I am creating the Shipment collection from grabbing the data from the database where every data row defines a Shipment line item which owns an Item and Stockroom. – CodenameCain Sep 23 at 23:08
0

As I understand it, your current implementation of Integrate would look something like this:

public void Integrate()
{
    if (this.Stockroom.Sys_ID != null) return;
    if (this.Stockroom.Name == null) return;
    this.Stockroom.Sys_ID = _someService.GetSysID(this.Stockroom.Name);
}

I don't see anything wrong with this approach; its simple, clear, and concise. The issue for you I think is that GetSysID might get called multiple times for the same stockroom name, resulting in extra calls that might not be needed.

One possible solution is to simply introduce a cache:

static Dictionary<string,string> _cache = new Dictionary<string,string>();

public void Integrate()
{
    if (this.Stockroom.Sys_ID != null) return;
    if (this.Stockroom.Name == null) return;
    this.Stockroom.Sys_ID = GetSysIDWithCache(this.Stockroom.Name);
}

protected string GetSysIDWithCache(string stockroomName)
{
    string result;
    if (_cache.TryGetValue(stockroomName, out result)) return result;

    var id = _someService.GetSysID(stockroomName);
    _cache.Add(stockroomName, id);
    return id;
}

This way you avoid introducing any complexity into the original logic, instead relying on the cache to avoid the extra calls.

0

Before bringing up the Heavy Artillery (Domain Events)

Consider having your objects handed to you by a factory. The factory can then ensure that no GPStockroom duplicates are created, because, comparing properties, it can keep a reference to any new GPStockroom instance that is created and if another GPShipment refers to the same GPStockroom, the factory will simply re-use the same reference. Then, whatever it is you do inside the Integrate() method will lead to the instantiation of this one shared GPStockroom reference and subsequent GPShipment objects will access this single already-instantiated reference, find the properties already there and no additional "reads" of the DataRow will have to be performed.

public class GPShipmentFactory
{
    //An additional suggestion is to make the GPShipment constructor
    //internal to force use of this factory instead of direct instantiation
    //of GPShipment objects.

    private Dictionary<string, GPStockroom> _uniqueStockrooms =
                                            new Dictionary<string, GPStockroom>();

    public static GPShipment CreateFromRow(DataRow row)
    {
        GPShipment shipment = new GPShipment();

        shipment.PopReceiptNumber = row.Field<string>("POPRCTNM");
        shipment.ReceiptLineNumber = Int32.Parse(row.Field<string>("RCPTLNNM"));
        shipment.QuantityShipped = row.Field<decimal>("QTYSHPPD");
        shipment.Item = new GPItem(row);

        //Check if the relevant row properties mean that it refers to an
        //identical Stockroom. For example if you want to use ID and the
        //field's name is STCKRMID:
        string stockroomID = row.Field<string>("STCKRMID");

        if (_createdStockrooms.ContainsKey(stockroomID)
        {
            //If a GPStockroom instance has already been created for this
            //ID, then there is no need to create a second one, just pass
            //the same reference.
            shipment.Stockroom = _createdStockrooms[stockroomID];
        }
        else
        {
            GPStockroom stockroom = new GPStockroom(row);
            shipment.Stockroom = stockroom;
            _uniqueStockrooms.Add(stockroomID, stockroom);
        }
    }
}

If you absolutely must, for some reason, have multiple instances of GPStockroom with identical Name and Sys_ID strings, another way to synchronize them is to store a Dictionary<string, GPShipment> somewhere, using the Name as your key. Then upon each GET operation (as you mean it), you can directly get all GPShipment instances with the same name and attach the same Sys_ID to them.

The last way to synchronize your objects is through events. However, I feel, from your cited design, that this is a bit overstretched; using events in this occasion will not add any flexibility, it will simply amend what appears to be a not-so-good design detail while adding enough complexity to far counterbalance any potential flexibility gains.

  • I will digest these options. Thank you and to the rest of you who have answered as well. Thank you for caring about the success of others. – CodenameCain Sep 24 at 14:53
0

Restating the problem as I understand it. I have a collection of Shipment objects that I’m iterating through and update a table.

A property in the Shipment class is named Stockroom.

A “target” table has a unique id column defining a stockroom row.

I have a collection of Shipments, not if I have. Some shipments relate to stockroom. No, all shipments relate to the Stockroom, if not, then why have the Stockroom property embedded in the Shipment class? (It really shouldn’t be embedded there due to separation of concerns or SRP, but I can’t change your design, so it is what it is.)

Some of the shipments have not been saved to a join table in the database. So, I’m going to get for the first time all the unique ids from the stock room table (and return that dataset back to the client. Ouch!)

Then I’ll make a save to the join table.

But I also need 1) inform some shipments in the collection what their unique id is and 2) avoid duplication in database calling and saving.

Proposed Solution:

Assumed Database Schema

Entity Table: Shipment

Join Table: Manifest (Shipment ID and Product ID, Invoice Number, Delivery Date)

Entity Table: Product (Your Line Items)

Join Table: Delivery (Manifest ID, Product ID and Stockroom ID, Quantity)

Join Table: Inventory (Stockroom ID and Product ID, Quantity)

Entity Table: Stockroom

Insert all rows into the delivery Table or equivalent. Some of delivery table rows just inserted will have a null stockroom id. Instead of a complex client side for-each iteration you can now deal directly with null stockroom id’s columns in your delivery table with a simple algorithm you create in your business rules.

Other suggested concepts you may want to follow up on: Single Responsibility Principal and Law of Demeter

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.