On Wed, Dec 7, 2016 at 1:04 PM, Nathan Guilty <ruby / e-solutions.re> wrote:
> Hi,
>
> Does someone can help me to refactor this piece of code :
>
>  18   def view_score
>  19     @board = Scoreboard.new
>  20     show(msg: '- The Best 3 SCORE -')
>  21     rows = []
>  22     rows << ['Name', 'Attempts', 'Time!']
>  23     rows << :separator
>  24     @board.stats.each { |s| rows << [s[:name], s[:tries], s[:time]] }
> unless     @board.stats.nil?
>  25     table = Terminal::Table.new rows: rows
>  26     table.align_column(1, :center)
>  27     table.align_column(2, :center)
>  28     puts "#{table}\n\n"
>  29   end

I noticed one odd thing: you create a new Scoreboard instance, then
never do anything with it (i.e. do not invoke any methods) yet you
seem to expect that @board.stats is not always nil (otherwise
iterating would not make sense). I wonder how those stats get filled
if at all.

In case #stats can change arbitrarily then storing the result in a
local variable could make sense to avoid race conditions, e.g.

@board.stats.tap {|st| st and st.each { |s| rows << [s[:name],
s[:tries], s[:time]] }}

Kind regards

robert

-- 
[guy, jim, charlie].each {|him| remember.him do |as, often| as.you_can
- without end}
http://blog.rubybestpractices.com/

Unsubscribe: <mailto:ruby-talk-request / ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk>