Behrang Saeedzadeh

A clean answer to the DailyWTF equality checking issue...

Friday, December 1, 2006 7:10:41 AM

This DailyWTF post about implementing a method that assigns a new value to a member variable and sets a dirty flag accordingly, actually does have a cleaner solution.

Here's the original code:

/**
    Set the value of the isRecordLocked attribute.
    @param newValue is the new isRecordLocked value.
*/
public void setIsRecordLocked(Boolean newValue) {
    // Update state if required.
    if (isRecordLocked != null) {
        if (newValue == null || !isRecordLocked.equals(newValue)) {
            context.setDirty(true);
        }
    }
    else if (newValue != null) {
        context.setDirty(true);
    }
    // Change the value.
    isRecordLocked = newValue;
}

And here's the cleaner implementation:

public void setIsRecordLocked(Boolean newValue) {

   if (newValue == null) {
      context.setDirty(isRecordLocked != null);
   } else {
      context.setDirty(!newValue.equals(isRecordLocked));
   }
   	
   isRecordLocked = newValue;

}

Actually, at the first glance I found myself in a mind freeze situation. But then in a a few minutes I came up with the cleaner solution.

Snap looks very promisingWhat keyboard is that?

Comments

Anonymous # Saturday, December 2, 2006 4:14:09 AM

Anonymous writes: The above is incorrect, since it sometimes calls setDirty(false) where the original code does not. It may not be valid to call setDirty(false) if for instance another method call on the object has already called setDirty(true). Then calling setDirty(false) would clear a dirty flag that should not be cleared.

Anonymous # Sunday, December 3, 2006 12:51:44 AM

Anonymous writes: public void setIsRecordLocked(Boolean newValue) { // Update state if required. if ( !( newValue != null && newValue.equals(isRecordLocked) ) ) { context.setDirty(true); } // Change the value. isRecordLocked = newValue; } although I'd probably write it public void setIsRecordLocked(Boolean newValue) { // Update state if required. if ( newValue != null && newValue.equals(isRecordLocked) ) { } else { context.setDirty(true); } // Change the value. isRecordLocked = newValue; }

Behrang Saeedzadehbehrangsa # Sunday, December 3, 2006 2:28:48 PM

Oops! You are right dude! Thanks for the correction

Anonymous # Sunday, December 3, 2006 8:10:41 PM

pedrow writes: Correct me if I'm wrong, but that still isn't right, is it? If both newValue and isRecordLocked are null, the WTF code doesn't call the function, but the post above does. How about public void setIsRecordLocked(Boolean newValue) { // Update state if required. if ( (newValue == isRecordLocked) || ((newValue != null) && newValue.equals(isRecordLocked))) { } else { context.setDirty(true); } // Change the value. isRecordLocked = newValue; } ?