One of the things I’ve learned over the years is to recognize when some code should be changed to be more maintainable. I’ll walk you through some code I saw during a pull request that we ended up refactoring to be more maintainable.
We were calling an external API that returns string “yes” and “no” for a bool value sometimes, and other times it sends a “true” and “false”. Obviously we wanted to accept both and convert them appropriately. The first version to accept these values was to have a property that checked the values of itself to let us know if it was true or false, it looked like this:
When we were mapping to our model, we were simply doing this:
When reviewing, we recognized two main issues. The first issue is that our values are duplicated, so if the API started sending us 1 and 0, then we might go update one area and forget to correct the other section. We could write some tests to let us know, but we didn't want to risk it, and of course there's a better way. There's always a better way. The other issue was that if one of the other fields that was sent to us was a "yes", but not actually a bool, then we'd be in trouble.
So, how did we solve the two issues? This is how:
With the fix, you'll notice that we're checking our destination model type now instead of the field's IsABooleanProperty. This fixes our second issue of getting an invalid bool. Our first problem is fixed by having the valid bool values a single time in a convert to bool method, and getting rid of the IsABooleanProperty. If you have other approaches to how to solve this problem or more examples of making code maintainable, please comment!
