Return to Answer

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, for such a simple == comparison, it's a bit much to extend a core class like string.

Also, Ruby convention is to skip the is_ prefix on interrogatory methods like those. The nil? method, for instance, isn't called is_nil?.

• 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 ifs (see update below)

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.

• 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 ifs (but see the update below)

• 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 ifs

Lastly: Naming (again). maintain_format is a bit off to me; you're applying a format, copying a format, or similar. Just a thought.

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.

• 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 ifs (see update below)

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.

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, for such a simple == comparison, it's a bit much to extend a core class like string.

Also, Ruby convention is to skip the is_ prefix on interrogatory methods like those. The nil? method, for instance, isn't called is_nil?.

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 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
• Everything gets downcased unless otherwise specified. So you only need to check if something is uppercase - everything else gets downcased by default.

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 similar. Just a thought.

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.

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, for such a simple == comparison, it's a bit much to extend a core class like string.

Also, Ruby convention is to skip the is_ prefix on interrogatory methods like those. The nil? method, for instance, isn't called is_nil?.

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 ifs

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 similar. Just a thought.

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.