HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Get persons full name from their First and Surname

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fullpersonsgetnamefirstandfromsurnametheir

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.

Code Snippets

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

Context

StackExchange Code Review Q#131774, answer score: 9

Revisions (0)

No revisions yet.