Given the following code, I am still refactoring it:
I’ve pulled out the FieldChanged function from the BeforeUpdate event already, see the TDD – 036 article for that. I still have way to many If blocks in that first function, and I don’t really like the readability of the second function. It is clear to me what it is doing by the name though: OnlyOneFieldIsNull, takes two values and returns a boolean.
All the function is really doing is checking both conditions where 1 is null and 2 is not null, or 1 is not null and 2 is null. Maybe just using Not instead of a true false check will be more readable:
It’s a little shorter. What if I add one more line to get rid of the parentheses and Or condition?
Actually, the above introduces possible logic errors because I’m trying to be clever. I think I’ll just create new variable names to document the conditions better.
Eh, I suppose I could make each of the variables functions which would then make this more readable as well. Ah, ok, I’ll do it.
Ok, now it’s obvious I think what’s going on. I also removed the retVal variable I usually use because I don’t think I’ll need to revisit these functions and if I rename them I still just need to rename one line. So far so good. Now I shall turn my attention to the bigger function:
So after the function to check if one field is null I check if both fields are null. That’s taking care of all the edge case null conditions. I think I can boil this function down to 2 lines. And edge case Null check and a non-null check. I will continue refactoring to my satisfaction next time.