Ted’s company hired a contract team to build an application. The budget eventually ran out without a finished application, so the code the contract team had produced was handed off to Ted’s team to finish.
This is an example of the Ruby code Ted inherited:
def self.is_uniqueness(mr_number)
out = false
mrn = PatientMrn.find_by_mr_number(mr_number)
if mrn
out = true
return mrn
end
return nil
end
The function is called is_uniqueness which is definitely some language barrier naming (is_unique is a more English way of wording it). But if we trace through the logic, this is just a wrapper around PatientMrn.find_by_mr_number– it returns an “mrn”.
So, the first obvious problem: this isn’t checking uniqueness in any way, shape or form.
Then there’s the whole check for a valid record- either we find a record or we return nil. But since find_by_mr_number is clearly returning something falsy, that doesn’t seem necessary.
And that is the default behavior for the Rails generated find_by methods- they just return nil if there are no records. So none of the checks are needed here. This whole method isn’t needed here.
Finally, there’s out. I have no idea what they were trying to accomplish here, but it smells like they wanted a global variable that they could check after the call for error statuses. If that was their goal, they failed in a few ways- first, returning nil conveys the same information. Second, global variables in Ruby get a $ sigil in front of them.
What the out variable truly represents is “do I want out of this codebase?” True. That is definitely true.
[Advertisement]BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!
Source: Read MoreÂ