CODEWITHSUNDEEP
c#refactoring
zrabzdn
zrabzdn

Reputation: 995

Refactor method with if statements

How refactor this method fragment with using predicate?

SetUpdateUserValue(User updateUser, User user)
{
    if (user.FirstName != null)
        updateUser.FirstName = user.FirstName;
    if (user.LastName != null)
       updateUser.LastName = user.LastName;
}

Upvotes: 0

Views: 89

Answers (3)

Andrew HON
Andrew HON

Reputation: 1

You can use reflection to copy properties from source object to destination object. here for your reference.

Upvotes: 0

Sudhakar Tillapudi
Sudhakar Tillapudi

Reputation: 26209

From Your comments: I don't like many if statements.

if you want to reduce the number of statements you can use null coalescing operator

Solution 1: using null coalescing operator

SetUpdateUserValue(User updateUser, User user)
{
    updateUser.FirstName = user.FirstName ?? updateUser.FirstName;
    updateUser.LastName = user.LastName ?? updateUser.LastName;        
}

Solution 2: using conditional (ternary) operator

SetUpdateUserValue(User updateUser, User user)
{
 updateUser.FirstName = user.FirstName!=null?user.FirstName:updateUser.FirstName;
 updateUser.LastName = user.LastName!=null?user.LastName:updateUser.LastName;
}

Upvotes: 2

peterchen
peterchen

Reputation: 41126

As is, the function is fine.

If you have long lists of these, you could consider

static void SetIfNotNull(ref string target, string source)
{
   if (source != null)
      target = source;
}



SetUpdateUserValue(User updateUser, User user)
{
    SetIfNotNull(ref updateUser.FirstName, user.FirstName != null);
    SetIfNotNull(ref updateUser.LastName,  user.LastName != null);
    ...
}

This would be a bit better for a long list, but still be prone to copy-and-paste errors.

Alternatively, you could use reflection to iterate the members (or a list of known members), if you are willing to take the performance hit.

Upvotes: 0

Related Questions