Get persons full name from their First and Surname

Posted on

Problem

My Person class has two properties; name and surname, and a function that returns the first name and the surname together.

Is there anything that can be done better? Anything that is done poorly?

public class Person
{
    private string Firstname { get; set; }
    private string Surname { get; set; }

    public Person(string firstname, string surname)
    {
        this.Firstname = firstname;
        this.Surname = surname;

    }
    public string GetFullName()
    {
        string fullname = String.Format("Firstname is: {0}, Surname is {1}", Firstname, Surname);
        return fullname;
    }
}

Solution

Naming

Whenever you have a simple method that starts with Get, it’s an indication that should should consider making it a property (in this case a read only one).

Unnecessary variable
The fullname variable is unnecessary, you might was well return simply the result of the String.Format.

Alternate for higher .Net versions

Depending on your .Net version, you might also want to use string interpolation instead. If you combine these three things then your GetFullName becomes:

public string FullName { 
                  get { return $"Firstname is: {Firstname}, Surname is {Surname}"; } 
              }

As an aside, usually you would simply expect the combined full name, rather than it being explicitly stated which is which, so you would have:

get { return $"{Firstname} {Surname}"; } 

or using the expression bodied auto property suggested by @Risky Martin you would end up with simply this:

public string FullName => $"{Firstname} {Surname}";

Parameter Validation

It may seem trivial for such a small class, but depending on the use case you should consider if you need to add validation to your constructor. Is it really valid to construct a Person with a null or empty string for example? As has been said by @Rick Davin, do you really want/need the setter for FirstName and Surname to be public, does it make sense for a Person object to change its name? If it does make sense, then again, you should consider if rather than using the auto property you should be performing validation in your set methods.

public class Person
{
    public Person(string firstname, string surname)
    {
        FullName = $"Firstname is: {firstname}, Surname is {surname}";
    }
    public string FullName { get; private set; }
}

Seeing you class I found you don’t need to generate FullName each time calling a method and you don’t need any private properties also. Keep it simple.

Leave a Reply

Your email address will not be published. Required fields are marked *