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.
2 Answers 2
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.
-
\$\begingroup\$ @BCdotWEB bad habits are not easy to overcome :(. thanks for this catch. \$\endgroup\$iSR5– iSR52020年07月31日 07:20:15 +00:00Commented Jul 31, 2020 at 7:20
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);
}
Explore related questions
See similar questions with these tags.