I prepared this question a long time ago in order to check knowledge of basic good practices.
Question #2
You have the following method which is responsible for updating a client in a database:
public void UpdateClient(
int id,
string name,
string secondname,
string surname,
string salutation,
int age,
string sex,
string country,
string city,
string province,
string address,
string postalCode,
string phoneNumber,
string faxNumber,
string country2,
string city2,
string province2,
string address2,
string postalCode2,
string phoneNumber2,
string faxNumber2,
...)
{
//...
}
Do you think that this code requires any refactoring? If yes, give your proposition. A database access technology doesn't matter in this question.
Answer #2
The basic problem with this method is that it has so many parameters. It is an error prone, difficult to maintain and use approach. I suggest to change the signature of this method in the following way:
public void UpdateClient(Client client)
{
//...
}
Where
Client is a class that models clients. It can look in the following way:
public class Client
{
public int Id { get; set; }
public string Name { get; set; }
public string Secondname { get; set; }
public string Surname { get; set; }
public string Salutation { get; set; }
public int Age { get; set; }
public string Sex { get; set; }
public Address MainAddress{ get; set; }
public Address AdditionalAdddress { get; set; }
/* More properties */
}
Address class contains details (country, city..) of addresses.
Comments #2
You may also write much more e.g.:
- It may be good to introduce enums for properties like 'Sex' which can take values only from the strictly limited range.
- UpdateClient method should inform a caller about the result of an update operation e.g. by returning a code.
However, the most important thing is to say that
UpdateClient method shouldn't have so many parameters. Personally, if I see a code as above I immediately want to reduce the number of parameters. This question seemed and still seems to be very easy, however not all candidates were able to answer it. Maybe it should be more accurate. For example, I should have stressed that a candidate should focus ONLY on available code. What do you think?