5
\$\begingroup\$

To test incoming data for string values that are too large for the "database" (MS Access), I could do this:

private void SaveToMSAccess(InventoryItem invItem, string dbContext)
{
    invItem = TruncateVerboseStringMembers(invItem);
    . . .
}

private InventoryItem TruncateVerboseStringMembers(InventoryItem invItem)
{
    const int DESC_MAX_LEN = 35;
    const int VENDOR_ID_MAX_LEN = 10;
    const int VENDOR_ITEM_MAX_LEN = 12;
    if (invItem.Description.Trim().Length > DESC_MAX_LEN)
    {
        invItem.Description = invItem.Description.Substring(0, DESC_MAX_LEN);
    }
    if (invItem.vendor_id.Trim().Length > VENDOR_ID_MAX_LEN)
    {
        invItem.vendor_id = invItem.vendor_id.Substring(0, VENDOR_ID_MAX_LEN);
    }
    if (invItem.vendor_item.Trim().Length > VENDOR_ITEM_MAX_LEN)
    {
        invItem.vendor_item = invItem.vendor_item.Substring(0, VENDOR_ITEM_MAX_LEN);
    }
    return invItem;
}

...where TruncateVerboseStringMembers() is always called, even though normally all conditions will fail (strings will not be too long), or I could do it like this:

const int DESC_MAX_LEN = 35;
const int VENDOR_ID_MAX_LEN = 10;
const int VENDOR_ITEM_MAX_LEN = 12;

private void SaveToMSAccess(InventoryItem invItem, string dbContext)
{
    if ((invItem.Description.Trim().Length > DESC_MAX_LEN) ||
        (invItem.vendor_id.Trim().Length > VENDOR_ID_MAX_LEN) ||
        (invItem.vendor_item.Trim().Length > VENDOR_ITEM_MAX_LEN))
    {
        invItem = TruncateVerboseStringMembers(invItem);
    }
    . . .
}

private InventoryItem TruncateVerboseStringMembers(InventoryItem invItem)
{
    if (invItem.Description.Trim().Length > DESC_MAX_LEN)
    {
        . . .

...where TruncateVerboseStringMembers() is only called when it needs to swing into action.

Or something fancier, where, if any strings are too long, I pass an array of bool to TruncateVerboseStringMembers().

Which is preferred?

\$\endgroup\$
1
  • 2
    \$\begingroup\$ You are still doing the double check. There is no good reason to check the length of all the strings, only to re-check all of them if any of them should be truncated. You only get duplication of code which means more code to maintain and in this case also more work for the code to do. \$\endgroup\$
    – Guffa
    Commented Feb 13, 2014 at 1:55

1 Answer 1

5
\$\begingroup\$

Making duplicate testing just to avoid a method call doesn't make much sense. Method calls are not that expensive. On the contrary, they can help make the code cleaner.

You can make a method that helps you clean up the strings, just pass in how long each string may be. I think that maked the code as DRY as possible:

private string TruncateString(string value, int maxLen) {
  value = value.Trim();
  if (value.Length > maxLen) {
    value = value.Substring(0, maxLen);
  }
  return value;
}

private InventoryItem TruncateVerboseStringMembers(InventoryItem invItem) {
  invItem.Description = TruncateString(invItem.Description, 35);
  invItem.vendor_id = TruncateString(invItem.vendor_id, 10);
  invItem.vendor_item = TruncateString(invItem.vendor_item, 12);
  return invItem;
}

This will give you the overhead of always calling the method for each string and always assigning the reference back, but those are very cheap operations. Compared to any database call, it's negligible.

If there is nothing in the string to trim, and nothing to truncate, you will actually be putting back the original string reference in the property, so there are no extra strings created in the usual case.

\$\endgroup\$
1
  • \$\begingroup\$ As the old cat in "Never Cry Wolf" said, "Good idea!" \$\endgroup\$ Commented Feb 12, 2014 at 21:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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