First off, I'd rename those two String
methods. You obviously had a plan in mind when you added them (i.e. that the strings you'd be checking would be 1 character long), but from a more general perspective, it's fraught with ambiguity.
For instance, is the string "Hello, world"
uppercase? Or is it lowercase? Is it sentence-case? Titlecase? So your methods, simple as they are, are perhaps too simple to make sense when applied outside the (narrow) usage you've tailored them for. As such, they probably shouldn't be global String
methods. Certainly, you don't need to extend a core class like String
just for a simple ==
comparison.
Also, Ruby convention is to not use an is_
prefix on interrogatory methods like those. The nil?
method, for instance, isn't called is_nil?
. Same for one?
, any?
, empty?
, include?
and so on.
As for your maintain_format
method, here's what came to mind:
- There's a
String#chars
method, which'll give you an array of chars - no need for split
- Instead of zipping, use a loop and use an index - characters in a string can be accessed just like elements in an array.
- And if you're going to loop, no need to loop further than the shortest of the two strings. Or, if you zip the arrays like you do right now, make sure you
break
the loop when one string or the other "runs out"
- You only need to check for uppercase. Everything else gets downcased by default. A character can only be upper or lowercase, so if it's not uppercase, you already know that it's lowercase; no need for extra
if
s (but see the update below)
I'd suggest something like
def maintain_format(template, string)
string = string.downcase # do this right away
template = template[0..string.length] # shorten the template, if necessary
template.chars.each_with_index do |char, index|
next unless char == char.upcase # skip lowercase chars
string[index] = string[index].upcase
end
string
end
Perhaps there's a smarter way to detect upper-/lowercase chars, but I couldn't quite think of one that'd be as simple and handle unicode etc.
Lastly: Naming (again). maintain_format
is a bit off to me; you're applying a format, copying a format, or something along those lines. (The title of your question also calls it "transfer" rather than "maintain". But transfer would suggest that one string loses its format, when it's applied to the other, so it's not quite right either.)
Ironically, it might make more sense to add this method to String
, since it's somewhat more generally applicable. Besides, a call like template.assimilate(other_string)
(besides sounding ominous, would make it much clearer which string is the template, and which string will be changed. Right now, it can quickly become confusing which argument is which. But again, better naming might alleviate that.
Update: As 200_success rightly points out in the comments, it's surprising that a string that's longer than the template string gets downcased rather than simply keep whatever case it had. And code shouldn't surprise.
Hence it might be better to do this:
def maintain_format(template, string)
string = string.dup # still want a duplicate to work on
template = template[0..string.length]
template.chars.each_with_index do |char, i|
string[i] = char == char.upcase ? string[i].upcase : string[i].downcase
end
string
end
You can still achieve the original behavior by simply downcasing the string yourself before passing it to the method.