Frozen string literals
Immutable data structures and purely functional code are always preferred, unless mutability and side-effects are required for clarity or performance. In Ruby, strings are always mutable, but there is a magic comment you can add to your files (also available as a command-line option for the Ruby engine), which will automatically make all literal strings immutable:
# frozen_string_literal: true
It is generally preferred to add this comment to all your files.
Word array "percent" literals
Ruby has special array literals for arrays of single-word strings that can make your code easier to read by reducing the amount of "syntax fluff" around the actual content.
The literal starts with the sigils %w or %W (think "word" or "witespace-separated"). %w behaves like a single-quoted string, i.e. does not perform interpolation and supports no escape characters other than \' and \\. %W behaves like a double-quoted string.
So, the beginning of your script could look like this:
# frozen_string_literal: true
## SAMPLE VARIANTS
sizes = %w[small medium large]
colors = %w[red green blue]
materials = %w[cotton linen]
As with all percent literals, you can freely choose the delimiter you want to use such that the delimiter does not occur inside the literal. For example, you can use | as the delimiter, ,, @ anything you want:
sizes = %w@small medium large@
colors = %w@red green blue@
materials = %w@cotton linen@
The first character after the w determines the delimiter. Delimiters come in two variants: paired and unpaired. With an unpaired delimiter, the same character ends the literal, as in the second example. With a paired delimiter, the corresponding closing delimiter ends the literal, e.g. when you start with <, you close with >, etc. See the first example.
Linting
You should run some sort of linter or static analyzer on your code. Rubocop is a popular one, but there are others.
Rubocop was able to detect all of the style improvements I pointed out above, and also was able to autocorrect all of the ones I listed.
I have set up my editor such that it automatically runs Rubocop with auto-fix as soon as I hit "save".
Here's what the result of the auto-fix looks like:
# frozen_string_literal: true
## SAMPLE VARIANTS
sizes = %w[small medium large]
colors = %w[red green blue]
materials = %w[cotton linen]
## ITERATE TO ALL VARIANTS
titles = []
sizes.each do |size|
colors.each do |color|
materials.each do |material|
## PUT THE VARIANT IN THE NEW ARRAY
titles.push("#{size} - #{color} - #{material}")
end
end
end
puts titles.inspect
puts foo.inspect
Kernel#p is the preferred debugging method. It does the same thing, but is more idiomatic, and is specifically designed for quick debugging (hence the one-character name).
So, the last line can simply be
p titles
Also, Kernel#puts returns nil, but Kernel#p returns its argument(s), so you can quickly chuck it into a long chain of expressions without changing the result.
Vertical whitespace
Your code could use some vertical whitespace to give the code more room to breathe. I would suggest at least separating the initialization at the beginning of the loop:
titles = []
sizes.each do |size|
colors.each do |color|
materials.each do |material|
## PUT THE VARIANT IN THE NEW ARRAY
titles.push("#{size} - #{color} - #{material}")
end
end
end
The "shovel" operator <<
Array#push is not idiomatic. More precisely, it is only idiomatic if you are using the array as a stack, then you would use Array#push and Array#pop, since those are the standard names for the stack operations.
The idiomatic way of appending something to something else is the shovel operator, in this case Array#<<, so that should be
titles << "#{size} - #{color} - #{material}"
Iterators
In Ruby, it is idiomatic to use high-level iterators. In your code, you are already using iterators instead of loops, so that is good. However, each is really the lowest-level of all the iterators. It is essentially equivalent to a FOREACH-OF loop. It has no higher-level semantics, and it relies on mutation and side-effects.
Whenever you have the pattern of "Initialize a result, loop over a collection appending to the result, return result", that is a fold. There are two implementations of fold in the Ruby core library, inject and each_with_object. inject is the more functional one, each_with_object is the more imperative one. So, for now, we will use each_with_object here, since the code is still pretty imperative, and that makes the relationship between the old and new code more clear.
As a general transformation,
accumulator = some_initial_value
collection.each do |element|
accumulator = do_something_with(accumulator, element)
end
becomes
accumulator = collection.inject(some_initial_value) do |accumulator, element|
do_something_with(accumulator, element)
end
or
collection.each_with_object(some_initial_value) do |element, accumulator|
do_something_with(accumulator, element)
end
In your case, it would look like this:
titles = []
sizes.each do |size|
colors.each do |color|
materials.each do |material|
## PUT THE VARIANT IN THE NEW ARRAY
titles << "#{size} - #{color} - #{material}"
end
end
end
becomes
titles = []
sizes.each_with_object(titles) do |size, titles|
colors.each_with_object(titles) do |color, titles|
materials.each_with_object(titles) do |material, titles|
## PUT THE VARIANT IN THE NEW ARRAY
titles << "#{size} - #{color} - #{material}"
end
end
end
Granted, this doesn't buy us much, actually the opposite. It starts to look slightly different though, when we move to a purely functional version without side-effects and mutation using Enumerable#inject:
titles = sizes.inject([]) do |acc, size|
colors.inject(acc) do |acc, color|
materials.inject(acc) do |acc, material|
## PUT THE VARIANT IN THE NEW ARRAY
acc + ["#{size} - #{color} - #{material}"]
end
end
end
Linter, revisited
Rubocop actually complains about my use of shadowing the outer acc with the inner acc.
I disagree. You should not be afraid to disable or re-configure rules in your linter to fit your style.
However, note that programming is a team sport. If you are modifying code, adopt the existing style. If you are part of a team, adopt the team's style. If you write open source code, adopt the project's style. If you start your own project, adopt the community's style (do not create your own style for your project, until your project is big and successful enough to have its own independent community).
Excursion: Generality of fold (inject / each_with_object)
When I wrote above that you can rewrite this iteration with inject or each_with_object, that was actually a tautological statement. I didn't even have to read the code to make this statement.
It turns out that fold is "general". Every iteration over a collection can be expressed using fold. This means, if we were to delete every method from Enumerable, except inject, then we could re-implement the entire Enumerable module again, using nothing but inject. As long as we have inject, we can do anything.
Iterators, take 2
So, what we did until now was replace the low-level iterator with a higher-level iterator.
However, we are still not done. What we are now doing is we take each three elements from our three collections, concatenating them, and putting it into a new collection. So, really, what we are doing is transforming each element (or triple of elements), or "mapping" each element to a new element.
This is called map and is also available in Ruby as Enumerable#map.
So, finally, our code looks like this:
titles = sizes.map do |size|
colors.map do |color|
materials.map do | material|
"#{size} - #{color} - #{material}"
end
end
end
This result is actually not quite right: we get a triply-nested array, because we have a triply-nested Enumerable#map.
We could Array#flatten the result, but there is a better way: Enumerable#flat_map:
titles = sizes.flat_map do |size|
colors.flat_map do |color|
materials.map do | material|
"#{size} - #{color} - #{material}"
end
end
end
What we did here, was to replace the general high-level iterator fold (which can do anything) with a more restricted, more specialized high-level iterator map. By using a more specialized iterator, we are able to better convey our semantics to the reader. Instead of thinking "Okay, so here we have an accumulator, and an element, and we do something with the element, and then append it to the accumulator … ah, I see, we are transforming each element", the reader just sees map and instantly knows that map transforms the elements.
Array methods
There is not much we can improve in the code by using iterators. However, there are many more methods in both the Enumerable mixin and the Array class.
So, let's take a step back and think about what we are actually doing here: we are constructing the Cartesian Product of the three arrays. And perhaps not surprisingly, there already is a method that computes a product of arrays, creatively named Array#product:
titles = sizes.product(colors, materials).map do |size, color, material|
"#{size} - #{color} - #{material}"
end
Array#join
As a last improvement, let's look at what the block is doing: it is "joining" the three variants together. And again, there is already a method which does that: Array#join:
titles = sizes.product(colors, materials).map do |variant|
variant.join(' - ')
end
Final result
So, in the end, the entire thing looks something like this:
# frozen_string_literal: true
## SAMPLE VARIANTS
sizes = %w[small medium large]
colors = %w[red green blue]
materials = %w[cotton linen]
titles = sizes.product(colors, materials).map do |variant|
variant.join(' - ')
end
p titles
Which I think is some nice-looking, easy to read, easy to understand code.
