patterncsharpModerate
Displaying a person's personal information with formatting
Viewed 0 times
formattingpersonwithdisplayingpersonalinformation
Problem
I am learning C# from a book, and there is an exercise as follows:
Write a program that has a person enter full name, age, and phone
number. Display this information with formatting. Also display the
person's initials.
I wrote some code and I feel that it could be improved, so I ask anyone to comment on it.
```
using System;
using System.Collections.Generic;
internal class Person
{
public string fname;
public string mname;
public string lname;
public int age;
public string phnumber;
public Person()
{
fname = KeyInput.keyIn("\n\nPlease enter your first name");
if (fname.ToLower() == "exit") return;
mname = KeyInput.keyIn("\nPlease enter your middle name");
lname = KeyInput.keyIn("\nPlease enter your last name");
while (true)
{
try
{
age = Convert.ToInt32(KeyInput.keyIn("\nPlease enter your age"));
break;
}
catch (ArgumentException)
{
Console.WriteLine("No value was entered");
}
catch (FormatException)
{
Console.WriteLine("You didn't entered a valid number");
}
catch (Exception e)
{
Console.WriteLine("Something went wrong with the conversion.");
}
}
phnumber = KeyInput.keyIn("\nPlease enter your phone number");
}
public void personDataDisplay()
{
Console.WriteLine("\nYour name is: " + fname + " " + mname[0] + "." + " " + lname);
Console.WriteLine("Your initials are: " + fname[0] + mname[0] + lname[0]);
Console.WriteLine("Your age is: " + age);
Console.WriteLine("Your phone number is: " + phnumber + "\n");
}
}
internal class KeyInput
{
public static string keyIn(string scrtext)
{
Console.WriteLine(scrtext);
string buffer = Console.ReadLine();
return buffer;
Write a program that has a person enter full name, age, and phone
number. Display this information with formatting. Also display the
person's initials.
I wrote some code and I feel that it could be improved, so I ask anyone to comment on it.
```
using System;
using System.Collections.Generic;
internal class Person
{
public string fname;
public string mname;
public string lname;
public int age;
public string phnumber;
public Person()
{
fname = KeyInput.keyIn("\n\nPlease enter your first name");
if (fname.ToLower() == "exit") return;
mname = KeyInput.keyIn("\nPlease enter your middle name");
lname = KeyInput.keyIn("\nPlease enter your last name");
while (true)
{
try
{
age = Convert.ToInt32(KeyInput.keyIn("\nPlease enter your age"));
break;
}
catch (ArgumentException)
{
Console.WriteLine("No value was entered");
}
catch (FormatException)
{
Console.WriteLine("You didn't entered a valid number");
}
catch (Exception e)
{
Console.WriteLine("Something went wrong with the conversion.");
}
}
phnumber = KeyInput.keyIn("\nPlease enter your phone number");
}
public void personDataDisplay()
{
Console.WriteLine("\nYour name is: " + fname + " " + mname[0] + "." + " " + lname);
Console.WriteLine("Your initials are: " + fname[0] + mname[0] + lname[0]);
Console.WriteLine("Your age is: " + age);
Console.WriteLine("Your phone number is: " + phnumber + "\n");
}
}
internal class KeyInput
{
public static string keyIn(string scrtext)
{
Console.WriteLine(scrtext);
string buffer = Console.ReadLine();
return buffer;
Solution
In designing classes we should try and confirm to the Single Responsibility Principle. Each class should only do one thing. Sometimes defining what makes a single responsibility is hard, but your
You should never have any significant logic in your constructor like you have here. Design your constructors to simply save any parameters and set up any default values. Any logic involving external classes, such as the Console, should definitely be placed in another method that is called once the object is constructed.
You should consider using the
Person class clearly does two things: it represents a person, and it questions the user via the console. This makes it difficult to reuse your Person class: what if you wanted to create a Person from a database, or a file or a web request? You would be better splitting this into a PersonQuestionnaire class which is responsible for creating a Person from a console input.You should never have any significant logic in your constructor like you have here. Design your constructors to simply save any parameters and set up any default values. Any logic involving external classes, such as the Console, should definitely be placed in another method that is called once the object is constructed.
You should consider using the
int.TryParse() method to read the age. Catching exceptions is slow (you will notice you ever write applications that are doing a lot of this type of thing), verbose, and sometimes unpredictable. You should definitely avoid catching the top level Exception type other than where you can do something meaningful: doing a lot of this will make your applications very difficult to debug.Context
StackExchange Code Review Q#23835, answer score: 12
Revisions (0)
No revisions yet.