10
\$\begingroup\$

There is a Name class with properties that represent different components that make up a person's name. A Name object requires a FirstName and Surname. All other fields are optional.

class Name
{
    public string FirstName { get; set; } = String.Empty;
    public string Surname { get; set; } = String.Empty;
    public string Rank { get; set; } = String.Empty;
    public string Suffix { get; set; } = String.Empty;
    public string NickName { get; set; } = String.Empty;
    public string MiddleName { get; set; } = String.Empty;

    public Name(string firstName, string surname)
    {
        this.FirstName = firstName;
        this.Surname = surname;
    }
}

Also, there is a NamesBuilder class that has a List<Name> collection. It has a GetListAsString method which iterates over the collection and builds a single string with the list of names:

class NamesBuilder
{
    List<Name> Names;

    public NamesBuilder()
    {
        Names = new List<Name>();
    }

    public NamesBuilder AddName(string firstName, string surname)
    {
        Names.Add(new Name(firstName, surname));
        return this;
    }

    public string GetListAsString()
    {
        StringBuilder sb = new StringBuilder();
        foreach (Name name in Names)
        {
            //add Title if exists
            if (name.Rank.Length > 0)
            {
                sb.Append(name.Rank);
                sb.Append(" ");
            }

            //add Firstname
            sb.Append(name.FirstName);
            sb.Append(" ");

            //add MiddleName if exists
            if (name.MiddleName.Length > 0)
            {
                sb.Append(name.MiddleName);
                sb.Append(" ");
            }

            //add NickName if exists
            if (name.NickName.Length > 0)
            {
                sb.Append((char)34);
                sb.Append(name.NickName);
                sb.Append((char)34);
                sb.Append(" ");
            }

            //add Surname
            sb.Append(name.Surname);

            //add Suffix if exists
            if (name.Suffix.Length > 0)
            {
                sb.Append(" ");
                sb.Append(name.Suffix);
            }

            //add new line
            sb.AppendLine();
        }

        return sb.ToString();
    }
}

This is called using method chaining:

static void Main(string[] args)
{
    NamesBuilder nb = new NamesBuilder()
        .AddName("James", "Kirk")
        .AddName("Montgomery", "Scott")
        .AddName("Nyota", "Uhura")
        .AddName("Leonard", "McCoy")
        .AddName("Christine", "Chapel");

    Console.WriteLine(nb.GetListAsString());
}

And this outputs:

James Kirk 
Montgomery Scott 
Nyota Uhura 
Leonard McCoy 
Christine Chapel

So the missing functionality is the ability to add optional Rank, Suffix, NickName and MiddleName details to each name. My initial thought was to change the AddName method to multiple optional parameters:

public NamesBuilder AddName(string firstName, string surname, string rank = "", string nickName = "", string middleName = "", string suffix = "")

However, this seems very verbose and inelegant especially if only the suffix needs to be added and all previous optional parameters are not applicable to that particular name.

My approach is to create new methods in the NamesBuilder class that would append those details to the last item added to the collection.

Here is the amended code of the caller illustrating this

static void Main(string[] args)
{
    NamesBuilder nb = new NamesBuilder()
        .AddName("James", "Kirk").SetRank("Capt").SetMiddleName("Tiberius")
        .AddName("Montgomery", "Scott").SetNickName("Scotty").SetRank("Lt Cdr")
        .AddName("Nyota", "Uhura").SetRank("Lt")
        .AddName("Leonard", "McCoy").SetSuffix("MD").SetNickName("Bones").SetRank("Lt Cdr")
        .AddName("Christine", "Chapel");

    Console.WriteLine(nb.GetListAsString());
}

And here is the updated NamesBuilder class:

class NamesBuilder
{
    List<Name> Names;

    public NamesBuilder()
    {
        Names = new List<Name>();
    }

    public NamesBuilder AddName(string firstName, string surname)
    {
        Names.Add(new Name(firstName, surname));
        return this;
    }

    public NamesBuilder SetRank(string rank)
    {
        Names[Names.Count - 1].Rank = rank;
        return this;
    }

    public NamesBuilder SetSuffix(string suffix)
    {
        Names[Names.Count - 1].Suffix = suffix;
        return this;
    }

    public NamesBuilder SetMiddleName(string middleName)
    {
        Names[Names.Count - 1].MiddleName = middleName;
        return this;
    }

    public NamesBuilder SetNickName(string nickName)
    {
        Names[Names.Count - 1].NickName = nickName;
        return this;
    }

    public string GetListAsString()
    {
        StringBuilder sb = new StringBuilder();
        foreach (Name name in Names)
        {
            //add Title if exists
            if (name.Rank.Length > 0)
            {
                sb.Append(name.Rank);
                sb.Append(" ");
            }

            //add Firstname
            sb.Append(name.FirstName);
            sb.Append(" ");

            //add MiddleName if exists
            if (name.MiddleName.Length > 0)
            {
                sb.Append(name.MiddleName);
                sb.Append(" ");
            }

            //add NickName if exists
            if (name.NickName.Length > 0)
            {
                sb.Append((char)34);
                sb.Append(name.NickName);
                sb.Append((char)34);
                sb.Append(" ");
            }

            //add Surname
            sb.Append(name.Surname);

            //add Suffix if exists
            if (name.Suffix.Length > 0)
            {
                sb.Append(" ");
                sb.Append(name.Suffix);
            }

            //add new line
            sb.AppendLine();
        }

        return sb.ToString();
    }
}

The output is now:

Capt James Tiberius Kirk
Lt Cdr Montgomery "Scotty" Scott
Lt Nyota Uhura
Lt Cdr Leonard "Bones" McCoy MD
Christine Chapel

I have never before used methods like this to alter the data of the most recent item added to a collection. It works and I think it looks much better than multiple optional parameters but I'd appreciate feedback.

\$\endgroup\$
9
\$\begingroup\$

Aside from optional arguments that may or may not be used, fluent API is very useful when it comes to open arguments, and it's also easy to expand and maintain.

Your approach is very good. You might need to add some restrictions though, in order to protect your class accessibility. Currently, Name can be changed from outside the NameBuilder which makes your design vulnerable for unwanted exceptions.

What you need is to disclose Name inside the class, and use it internally, it doesn't need to be exposed, and restrict its access to only used through NameBuilder class.

Your current API is fine if it won't have much functionalities to add, but if you have some other requirements (other than adding names), I would suggest to wrap current work inside an internal class (inside the NameBuilder) which would handle the required functionalities. For instance, you might implement a class to handle adding new names, and another to process some actions such as formatting. All of which would be under the main class which would be the container to contain and navigate between them.

GetListAsString() why not ToString()?

since you've already defaulted your properties to string.Empty you can override ToString() on Name class to have this :

public override string ToString()
{
    return $"{Rank}{FirstName}{MiddleName}{NickName}{Surname}{Suffix}".Trim();
}

then in your NameBuilder class do this :

private string Add(string text)
{
    return $"{text} ";
}

public NamesBuilder SetRank(string rank)
{
    _current.Rank = Add(rank);
    return this;
}

public override string ToString()
{
    return string.Join(Environment.NewLine, Names);        
}

Now, just call ToString() to get the concatenated string.

the Add(string text) would just add a tailing space.

Lastly, there is no single validation used. You should validate each string, and make sure it fits your requirement before assign it.

\$\endgroup\$
1
  • \$\begingroup\$ @BCdotWEB bad habits are not easy to overcome :(. thanks for this catch. \$\endgroup\$ – iSR5 Jul 31 '20 at 7:20
1
\$\begingroup\$

If you're using C# 8.0++ you can use the "index from end" operator instead of Count - 1

Names[^1].Suffix = suffix;

I would make a private property Current of NameBuilder to reach the last name object by:

private Name Current => Names.Count > 0 ? Names[^1] : throw new InvalidOperationException("No names in Builder");

and maybe a method that sets a member via a delegate:

private NamesBuilder SetValue(Action<Name> setter)
{
  setter(Current);
  return this;
}

Then the Set_X()-methods could be reduced to:

public NamesBuilder SetRank(string rank) => SetValue(n => n.Rank = rank);
public NamesBuilder SetSuffix(string suffix) => SetValue(n => n.Suffix = suffix);
public NamesBuilder SetMiddleName(string middleName) => SetValue(n => n.MiddleName = middleName);
public NamesBuilder SetNickName(string nickName) => SetValue(n => n.NickName = nickName);

if FirstName and SurName are mandatory, you should maybe make them readonly:

public string FirstName { get; }
public string Surname { get; }

and check their values in the constructor:

public Name(string firstName, string surname)
{
  this.FirstName = !string.IsNullOrWhiteSpace(firstName) ? firstName : throw new ArgumentException("Must have a valid value (at least one character)", nameof(firstName));
  this.Surname = !string.IsNullOrWhiteSpace(surname) ? surname : throw new ArgumentException("Must have a valid value (at least one character)", nameof(surname));
}

You can override ToString() in Name as ISR5 also suggest, but I would avoid appending a space char at the end of the value. Instead I would do like this:

public override string ToString()
{
  string[] parts = 
  { 
    Rank, 
    FirstName, 
    string.IsNullOrWhiteSpace(NickName) ? null : $"\"{NickName}\"", 
    Surname, 
    NickName, 
    Suffix, 
  };
  return string.Join(" ", parts.Where(p => !string.IsNullOrWhiteSpace(p)));
}

where the order of the parts in parts correspond to their order in the result string.

Then GetListAsString() - which should maybe be renamed to GetNamesAsString() - or just ToString() as ISR5 suggest - could look like:

public string GetNamesAsString()
{
  return string.Join(Environment.NewLine, Names);
}
\$\endgroup\$

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.