Story time!
def myslice(begin_tag, end_tag)
string = "@note[/(?<=" + begin_tag.to_s + ")(.*?)(?=" + end_tag.to_s
+ ")/m].to_s)"
result = eval(string)
cutupa = "@note.slice!(" + begin_tag.to_s + ")"
cutupb = "@note.slice!(" + result.to_s + ")"
cutupc = "@note.slice!(" + end_tag.to_s + ")"
eval(cutupa)
eval(cutupb)
eval(cutupc)
return result
end
Starting at the top.
def myslice(begin_tag, end_tag)
The method name should be myslice! since it makes permanent alterations to the string.
string = "@note[/(?<=" + begin_tag.to_s + ")(.*?)(?=" + end_tag.to_s
+ ")/m].to_s)"
result = eval(string)
Bunch of nitpicks. The extraneous ) was already mentioned.
string and result are poor variable names. Neither communicate what we are doing very clearly. A good idea is to name our variables according to what they are supposed to hold. We eventually want result to return the sliced out section of the @note, so as an example we could rename it to tagged_section to communicate this clearly.
return result
return tagged_section
I like it better anyway.
That out of the way, the code itself is a little weird. I know what it's trying to do, and we'll fix it in a bit, but let's go over a couple points first. To begin with, the code is hacked up into a couple lines to keep it short so it displays in the rgss editor without the need for horizontal scrolling. That gets points for effort, but in this case it isn't increasing readability very well. We could try again.
result = eval("@note[/(?<=" + begin_tag.to_s + ")(.*?)(?=" +
end_tag.to_s + ")/m].to_s")
Or we could forget about it and say there are times when we can live with horizontal scrolling.
result = eval("@note[/(?<=" + begin_tag.to_s + ")(.*?)(?=" + end_tag.to_s + ")/m].to_s")
Either way, these eliminate the need for a local variable, which is usually a good thing.
Speaking of variables, we don't need to .to_s everything. We need to .to_s the slice method (or [] method, in this case), because we don't know if we are getting a string or a nil. But we do know that begin_tag and end_tag are going to be stringsâ€â€unless the user calls myslice! with the wrong parameters. We could do something about it by raising an error, but I'm more inclined to say it's their problem.
result = eval("@note[/(?<=" + begin_tag + ")(.*?)(?=" + end_tag + ")/m].to_s")
As a bonus, this now fits onto one line in the rgss editor.
Finally, using eval() to return something is a little iffy and probably not the best idea in this case. It could work, but it's not the easiest or the most robust thing to do. Instead, we can alter our regular expressions yet again, with a new feature; the brilliant #{...}. When ruby sees an #{...} inside of a regular expression, it does a little alteration to the expression before using it. Here's a few examples.
/#{1+5}/ is the same as /6/.
/#{"hi"}/ is the same as /hi/.
hi = "pie"; /#{hi}/ is the same as /pie/.
Cutting a long story short, we can do this:
/(?<=#{begin_tag})(.*?)(?=#{end_tag})/m
Now we don't have to go to all the work of constructing a string and then evaluating it. Instead, we can replace all three lines of the original code with this:
tagged_section = @note[/(?<=#{begin_tag})(.*?)(?=#{end_tag})/m].to_s
Moving on.
cutupa = "@note.slice!(" + begin_tag.to_s + ")"
cutupb = "@note.slice!(" + result.to_s + ")"
cutupc = "@note.slice!(" + end_tag.to_s + ")"
eval(cutupa)
eval(cutupb)
eval(cutupc)
As mentioned, all of that could be turned into this:
@note.slice!(begin_tag)
@note.slice!(tagged_section)
@note.slice!(end_tag)
Notice we also removed the .to_s methods (we already .to_s tagged_section when we assigned it in the above code, so it doesn't need .to_s again). The new version is shorter, has no local variables, and is easier to read.
Making all our changes, our code now looks something like this.
def myslice!(begin_tag, end_tag)
tagged_section = @note[/(?<=#{begin_tag})(.*?)(?=#{end_tag})/m].to_s
@note.slice!(begin_tag)
@note.slice!(tagged_section)
@note.slice!(end_tag)
return tagged_section
end
Now, there's one more nitpicky problem. myslice! is essentially a new slice!, and it should behave almost identically, but it doesn't.
slice! will either delete some of the string if it can, and return the deleted portion, or not delete anything and return nil.
myslice! will delete some of the string if it can, and return some of the deleted portion (but not the tags), or (probably) not delete anything and return a blank string.
Now, not returning the entire deleted portion is okay. If we wanted to do that we wouldn't be making a new slice! method in the first place. But we should probably return nil if we aren't deleting anything, so we match the design of the original slice! method.
def myslice!(begin_tag, end_tag)
tagged_section = @note[/(?<=#{begin_tag})(.*?)(?=#{end_tag})/m]
@note.slice!(begin_tag)
@note.slice!(tagged_section.to_s)
@note.slice!(end_tag)
return tagged_section
end
All we did was move the .to_s. So now, if there is no tagged section in the @note, our tagged_section variable is nil, meaning our whole method returns nil. However, we still need to make sure that tagged_section is a string when we slice! it, so we add it back on the relevant line.
All in all, I think it should be much closer to working. I didn't test it though.