I’m trying to create a function that will take input in the form “a12345678” and output text in the form “a12-34-5678” if and only if the input text is not in the form “adam smith” The goal is to allow my users to enter either names separated by a space, or the student id number into a single search field and have the results be returned based on the format of the query.
I came up with the following code in my student model
def self.insert_dashes(term)
if /^([A-Z]|[a-z]|\d|-){11}$/.match(term)
term.insert 3, ‘-’
term.insert 6,‘-’
end
end
def self.find_record(rec)
if /^([A-Z]|[a-z]|\d|-){11}$/.match(rec)
student=Student.where(:studentID=>rec).all
elsif /^([A-Z\s]|[a-z\s])+$/.match(rec)
split=rec.split ’ ',2
f_name=split.first
l_name=split.second
student=Student.where(:fname=>f_name,:lname=>l_name).all
else
bar=insert_dashes(rec)
find_record(bar)
end
end
If I try to enter a string of the for “a12345678” I am greeted with the following error:
SystemStackError (stack level too deep):
app/controllers/students_controller.rb:19:in `find_student’
Have a look at the Rails Guide on Debugging to find how to use
ruby-debug to break into your code and inspect variables and follow
flow. Then you can debug the method yourself to find what is going
wrong. That way you will learn a lot for the future.
I'm wondering if there's not some whitespace getting added on to the
search term - unless you're stripping it elsewhere, trailing
whitespace will cause this method to run indefinitely - the first time
around insert_dashes will return nil (since it doesn't match the
*exact* character count and the match is fixed at both ends), and that
will then just keep going around.
DRY is nice and all, but here I think you're trading readability (not
to mention halting!) for one repeated line of code. Given that you
*know* what format the argument will be in after calling
insert_dashes, wouldn't it make more sense to just repeat
'Student.where(:studentID=>rec).all' there?
Couple unrelated things:
- I'm not sure why you're alternating amongst different character
classes - this is exactly identical to adding the alternatives *to*
the character class. So:
/^([A-Z]|[a-z]|\d|-){11}$/
can be shortened to:
/^[A-Za-z\d\-]{11}$/
and similarly for the other pattern.
- the code as written will die with StackLevelTooDeep when a user puts
in a name with punctuation (an apostrophe or a hyphen, for instance).
This is most likely a bug.
- the name of this function isn't quite right - you're returning an
*array* from a function named find_record (singular). It would
probably work better to use .first instead of .all in the code above,
or to rename the function to find_records.
The first match will match the pattern "a12-34-5678" and get the
records based on that as the studentID key.
The second match matches "a12345678" which is converts into the
correct format and does the same.
The final part assumes that if the input string does not match either
pattern then is *should* be a name in the format "john smith" (that is
to say a string with some whitespace in it) and will get all the
student records based on that.
To be honest you should check that last assumption incase someone
inputs "john", "dr john alan" or "john williams III".
