0

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;
}
Stefan Over
  • 5,851
  • 2
  • 35
  • 61
zrabzdn
  • 995
  • 2
  • 14
  • 33

3 Answers3

2

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;
}
Sudhakar Tillapudi
  • 25,935
  • 5
  • 37
  • 67
  • 2
    This behaves differently: the original code does not modify updateUser if the respective user member is null. – peterchen Mar 06 '14 at 14:21
  • @peterchen: yes, updated my answer. Thanks for yur valuable comment. – Sudhakar Tillapudi Mar 06 '14 at 14:23
  • This code have doubles: updateUser.FirstName = user.FirstName ?? updateUser.FirstName; updateUser.LastName = user.LastName ?? updateUser.LastName; – zrabzdn Mar 06 '14 at 14:27
  • 1
    @zrabzdn: yes it has becaue if the value is `null` it shouldnot assign anything means it should reassign the same value what it has now. i don't think you can get much smaller than this :( – Sudhakar Tillapudi Mar 06 '14 at 14:34
  • Agree with Sudhakar. This is as good as can be reasonably expected. Don't use reflection for this when writing production code. Instead, encapsulate this behavior in a class. Use good OO, not expensive and difficult to read language features. – P.Brian.Mackey Mar 06 '14 at 15:21
0

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.

peterchen
  • 40,917
  • 20
  • 104
  • 186
0

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

Community
  • 1
  • 1
Andrew HON
  • 1
  • 1