Drawing colored circles using Ruby and Tk

Question

I am a high-school freshman who is kinda new to Ruby, and I am doing a small project on Ruby. One of the big things that I want to get out of this project is how to follow the "Ruby standards" that programmers should follow. Being as new as I am, I have no clue what I should/shouldn't do with this program. Can anybody tell me what I could do to improve it to fit the community's standards?

require 'tk'
$point_A = [0,0]
$point_B = [750,750]
$rate = 1.5
$i=0
circs=Array.new
def before_drawing()
 $point_A = []
 temp_a = $point_B[0]**1/$rate
 temp_b = $point_B[1]**1/$rate
 $point_A << temp_a
 $point_A << temp_b
end
def after_drawing()
 $point_B = []
 $point_B = $point_A
end
canvas = TkCanvas.new(:width=>800, :height=>800).pack('fill' => 'both', 'expand'=>true)
while $i<10 do
 before_drawing()
 circs[$i] = TkcOval.new(canvas, $point_A, $point_B)
 if $i%2==0 then
 circs[$i][:fill] = 'blue'
 else
 circs[$i][:fill] = 'red'
 end
 after_drawing()
 $i+=1
end
Tk.mainloop

Answer 1

You should define arrays with [] not Array.new

circs should be $circs in case you wrap your loop in some function.

Before drawing can be turned into this :

def before_drawing()
 temp_a = $point_B[0] ** 1 / $rate
 temp_b = $point_B[1] ** 1 / $rate
 $point_A = [temp_a, temp_b]
end

You should turn $i into a local variable for the loop. There is no need for it to be global.

Then replace the loop with upto.

0.upto(10) do |i|
 before_drawing()
 circs[i] = TkcOval.new(canvas, $point_A, $point_B)
 # As suggested using ternary operator
 # circs [i] [:fill] = i % 2 == 0 ? 'blue' : 'red'
 if i % 2 == 0 then
 circs[i][:fill] = 'blue'
 else
 circs[i][:fill] = 'red'
 end
 after_drawing()
end

You seem to use too many global variables. My advice would be to prefer local variables whenever you can (Like i in the loop). All your global variables can also be made local to the loop (or function in case you wrap the loop in some function).

• fr00ty_l00ps: Alrighty, that makes sense.. I had no idea that something like upto existed, and I was afraid to use x.times{}
• Nakilon: I would recommend to change if i % 2 == 0 then circs[i][:fill] = 'blue' else circs[i][:fill] = 'red' end to circs[i][:fill] = (i % 2 == 0 ? 'blue' : 'red') or even circs[i][:fill] = %w{blue red}[i % 2].
• Aleksandar: Using the ternary operator isn't a bad idea.

Answer 2

Some notes:

1. Use tabspace=2.

2. Don't use global variables. When programming you should use functions in the same sense than you do in maths. That's a real function: f(x, y) = x + y, note that it takes arguments and returns some output (no globals, no states, no updates to variables outside the function).

3. Ruby is a OOP language, so we usually define a class (or module) to contain our code.

4. Don't overuse statements, use expressions. This code uses statements: x = []; x << 1; x << 2, this one uses expressions: x = [1, 2].

5. You are writing a loop where the output is the input of the next iteration. That can be written with Enumerable#inject (this method is somewhat difficult to grasp at first, study the docs carefully).

A more idiomatic Ruby approach would be:

require 'tk'
class Example
 def initialize(options = {}) 
 @rate = options[:rate] || 1.5
 @start_point = options[:start_point] || [750, 750]
 @canvas_size = options[:canvas_size] || [800, 800]
 end
 def run
 canvas = TkCanvas.new(:width => @canvas_size[0], :height => @canvas_size[1])
 canvas.pack('fill' => 'both', 'expand' => true)
 1.upto(10).inject(@start_point) do |point, index|
 # get_next_point is a one-liner and could be written here,
 # but let's show how to use arguments to call functions/methods.
 point2 = get_next_point(point, @rate)
 circle = TkcOval.new(canvas, point, point2)
 circle[:fill] = (index % 2) == 0 ? "red" : "blue"
 point2
 end
 Tk.mainloop
 end
 def get_next_point(point, rate)
 [point[0] / rate, point[1] / rate]
 end
end
if __FILE__ == 0ドル
 example = Example.new(:rate => 1.5, :start_point => [750, 750])
 example.run
end

• user229044: Ruby may be OOP, but it's also a scripting language designed for building small purpose-built scripts. There is no need to shoe-horn in a class definition for such a small program, but if this is meant to be the foundation of a larger app then it's definitely a good idea.
• tokland: @meagar, I agree, but as you say I tried to show good practices for medium/large scripts.

Answer 3

This is slightly late, but I feel like the existing answers are somewhat over-wrought.

A few points:

• 10.times, as others have mentioned
• You're not using ** correctly: x ** 1 / rate is the same as (x ** 1) / rate, and x ** 1 equals x. So, things are cleaned up right away by replacing ** 1 / rate with / rate
• You can pass :fill directly to the constructor of TkcOval, meaning you don't need to store your circles at all
• If you do need to store the circles, you can use circles = (0..9).map { |i| ...} instead of 10.times and return the circle from the block
• You can compute color on one line using the ternary operator, or, better yet, store the colors in an array (colors = %w(blue red)) which can ban indexed by i % 2
• Because we're dealing with maths, I prefer to store the points outside an array as (x1, y1) and (x2, y2). I think this makes things clearer than using arrays. Clearer still would be using points with .x and .y members like p1.x, p1.y, but that isn't supported
• There is no need for your before/after methods, because they're doing next to nothing. They should be written as single lines of code.

Here's the results, about 10 lines of extremely concise and idiomatic code:

require 'tk'
canvas = TkCanvas.new(:width => 800, :height => 800).pack('fill' => 'both', 'expand' => true)
x1, y1, rate = 750, 750, 1.5
colors = %w(blue red)
10.times do |i|
 x2, y2 = x1 / rate, y1 / rate
 TkcOval.new(canvas, [x1, y1], [x2, y2], :fill => colors[i % 2])
 x1, y1 = x2, y2
end
Tk.mainloop

Note that we could get even shorter and ditch x2/y2, but I feel this starts to verge on code-golf rather than simply writing concise code:

require 'tk'
canvas = TkCanvas.new(:width => 800, :height => 800).pack('fill' => 'both', 'expand' => true)
x, y, colors, rate = 750, 750, %w(blue red), 1.5
10.times do |i|
 TkcOval.new(canvas, [x, y], [x /= rate, y /= rate], :fill => colors[i % 2])
end
Tk.mainloop

Furthermore, note some style issues:

• two spaces for indentation, no more no less
• don't prefix your variables with $, you're making global variables needlessly.