Transfer the format of an old string to a new string

Question

First, I've extended the String class to detect uppercase and lowercase characters:

class String
 def is_uppercase?
 self == self.upcase 
 end
 def is_lowercase?
 self == self.downcase
 end
end

I then use these instance methods to fulfil my main goal of transferring the format of an old string to a new string:

def maintain_format(instring, outstring) 
 instring_array = instring.split(//)
 outstring_array = outstring.split(//)
 both_arrays = instring_array.zip(outstring_array)
 both_arrays.each do |character|
 if character[0] == nil || character[0].is_lowercase?
 character[1].downcase! if character[1]
 elsif character[0].is_uppercase?
 character[1].upcase! if character[1]
 end
 end
 outstring_array.join("")
end

It works well and does exactly what I want:

2.0.0p247 :002 > maintain_format('jimmy','Jam')
 => "jam" 
2.0.0p247 :003 > maintain_format('Jimmy','xam')
 => "Xam" 
2.0.0p247 :004 > maintain_format('realLylongname','shorter')
 => "shorTer" 
2.0.0p247 :005 > maintain_format('rSeHorteR','muchmuchmuchlongerfoos')
 => "mUcHmuchMuchlongerfoos" 

However, what I love about ruby is how it is sort self-documenting if you use sensible variable and method names and hash keys. With this code it is a bit hard to follow regarding the 1character[0]character[1]` stuff. I attempting it with hashes but got in a bit of a mess. How would you tackle this problem?

Also interested in any ruby tricks to make this take up less lines.

Note this is part of a rails project, so all rails methods are available to me.

Answer 1

My common advise is to avoid accumulator variables.

def maintain_format template, string
 string.chars.zip(template.chars).map{ |orig, char|
 char ? char == char.upcase ? orig.upcase : orig.downcase : orig
 }.join
end
p maintain_format "qWERty", "asDFGhJj"
>> "aSDFghJj"

Note char ? ... : orig since zip gives nils instead of second array element if it's not available (if template is too short).

• Flambino: +1 for avoiding the accumulator. But that double ternary is kinda hairy. It's not super complex or anything, but it's unnecessarily terse and harder to read.

Answer 2

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 ifs (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.

• 200_success: I'd expect the part of the string that extends beyond the end of template to have its case preserved, not forcibly downcased.
• Flambino: @200_success As would I. But as far as I can tell from OP's code, case isn't preserved for the "leftover" string... Regardless, you're right; it's unexpected. I'll add a note - thanks
• Starkers: @Flambino could you explain the 5th line please? ` string[i] = char == char.upcase ? string[i].upcase : string[i].downcase` Literally all of it. I've got a good idea but I'd want some validation :)
• Flambino: @Starkers It's just an assignment with a ternary, i.e. if...else written in one line. You can spell it out in Ruby as string[i] = (if char == char.upcase then string[i].upcase else string[i].downcase end).