You might be right, I might be crazy...
Author: John on April 01, 2016
...but the result is the same.
Recently while working on a feature request, I found a method needed to be changed due to the nature of the request. We have a upload bid feature which allows the user to upload a csv file with their bids instead of navigating the view manually. If there is an error, we display a message but the Submit button was still active. We needed to disable this button when there were errors present during the file upload process.
Ok, let us take a look at the original code:
# original code def disable_submit_button? !zero_quantity_bids? && all_bids_confirmed? && @file_upload_errors end
The problem with the original code is that we check for file upload errors last and we are using the && operator, checking for the result on all three method calls. To address the issue of disabling the Submit button if there were file upload errors, I made the following update:
# my solution def disable_submit_button? return true if @file_upload_errors !zero_quantity_bids? && all_bids_confirmed? end
While the solution above worked fine, and it read well to me, my team lead wanted me to try a different solution. The result is the same, but take a look and the finalized version:
# finalized version (by team lead) def disable_submit_button? @file_upload_errors || (!zero_quantity_bids? && all_bids_confirmed?) end
The finalized version does reduce a line of code. For me, my version reads better but I fully understand and can appreciate the finalized verison. I would love to hear what your thoughts are? Perhaps you would like to suggest another solution?
Learn Something New Everyday
Last Edited by: John on April 01, 2016