3
\$\begingroup\$

I am accepting a form from the user with a bunch of inputs and I am constructing a string from those inputs. I am trimming each input and 2 of the inputs (GID & Clone) are not required fields so I'm doing null checks before building the string to avoid null object errors.

I wrote this method and it works, but I was wondering if there is a less repetitive way of doing this.

private string ConstructSeedReference(SeedViewModel model)
{
    string SeedRefId;

    if (!String.IsNullOrEmpty(model.Seed.GID) && !String.IsNullOrEmpty(model.Seed.Clone))
    {
        SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}{7}{8}",
        model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId, model.Seed.GID.TrimEnd(), model.Seed.Clone.TrimEnd());
        return SeedRefId;
    } 
    else if (!String.IsNullOrEmpty(model.Seed.GID) && String.IsNullOrEmpty(model.Seed.Clone))
    {
        SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}{7}",
        model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId, model.Seed.GID.TrimEnd());
        return SeedRefId;
    }
    else if (String.IsNullOrEmpty(model.Seed.GID) && !String.IsNullOrEmpty(model.Seed.Clone))
    {
        SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}{7}",
        model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId, model.Seed.Clone.TrimEnd());
        return SeedRefId;
    }
    else
    {
        SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}",
        model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId);
        return SeedRefId;
    }
}
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$
String.Format("{0}{1}{2}{3}{4}{5}{6}", ...);

Since you are just doing concatenation, string.Concat is more appropriate here.

if (!String.IsNullOrEmpty(model.Seed.GID) && !String.IsNullOrEmpty(model.Seed.Clone))
{
    SeedRefId = String.Format("{0}{1}{2}{3}{4}{5}{6}{7}{8}",
    model.Customer.CustomerCode.TrimEnd(), model.Seed.CollectionCodeId.TrimEnd(), model.Seed.SpeciesId.TrimEnd(),
    model.Seed.Zone.TrimEnd(), model.Seed.Elevation.TrimEnd(), model.Seed.ColYear.TrimEnd(), model.Seed.OrchardId, model.Seed.GID.TrimEnd(), model.Seed.Clone.TrimEnd());
    return SeedRefId;
} 

Between your ifs and else, there is a lot of repeated code. And, we can observe a pattern there:

  • The first 7 elements are always present.
  • The last 2 elements are present if it is not null/empty
  • The elements are always in the same order.

The method can be reduced to this:

private string ConstructSeedReference(SeedViewModel model)
{
    var reference = string.Concat(
        model.Customer.CustomerCode.TrimEnd(), 
        model.Seed.CollectionCodeId.TrimEnd(), 
        model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(), 
        model.Seed.Elevation.TrimEnd(), 
        model.Seed.ColYear.TrimEnd(), 
        model.Seed.OrchardId);

    if (!string.IsNullOrEmpty(model.Seed.GID))
    {
        reference += model.Seed.GID.TrimEnd();
    }

    if (!string.IsNullOrEmpty(model.Seed.Clone))
    {
        reference += model.Seed.Clone.TrimEnd();
    }

    return reference;
}

EDIT: Just reread your question, if your concern is only about the NullReferenceException. You can simply use null-coalescing operator ?.:

private string ConstructSeedReference(SeedViewModel model)
{
    return string.Concat(
        model.Customer.CustomerCode.TrimEnd(),
        model.Seed.CollectionCodeId.TrimEnd(),
        model.Seed.SpeciesId.TrimEnd(),
        model.Seed.Zone.TrimEnd(),
        model.Seed.Elevation.TrimEnd(),
        model.Seed.ColYear.TrimEnd(),
        model.Seed.OrchardId,
        model.Seed.GID?.TrimEnd(),
        model.Seed.Clone?.TrimEnd());
}
\$\endgroup\$

Your Answer

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

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