# Drawing colored circles using Ruby and Tk

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**1/$rate temp_b =$point_B**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


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 ** 1 / $rate temp_b =$point_B ** 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). • Alrighty, that makes sense.. I had no idea that something like upto existed, and I was afraid to use x.times{} Sep 24, 2012 at 14:07 • 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]. Jan 4, 2013 at 20:07 • Using the ternary operator isn't a bad idea. Jan 5, 2013 at 10:38 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, :height => @canvas_size) 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 / rate, point / rate] end end if __FILE__ ==$0
example = Example.new(:rate => 1.5, :start_point => [750, 750])
example.run
end

• 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. Jan 6, 2013 at 3:18
• @meagar, I agree, but as you say I tried to show good practices for medium/large scripts. Jan 6, 2013 at 9:52

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.