HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Drawing colored circles using Ruby and Tk

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
coloredcirclesdrawingusingrubyand

Problem

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

Solution

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).

Code Snippets

def before_drawing()
    temp_a = $point_B[0] ** 1 / $rate
    temp_b = $point_B[1] ** 1 / $rate
    $point_A = [temp_a, temp_b]
end
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

Context

StackExchange Code Review Q#15806, answer score: 5

Revisions (0)

No revisions yet.