Hey John,
Please don’t get angry with me for what I’ll tell you.
The real problem with your code is the “foundation”.
You’re trying to build a good house over pure beach sand.
Someday it’ll fall.
I asked about the blog because it says (almost as an ADVICE of DANGER):
“I started playing around with Ruby recently and it is a really fun
language, albeit the syntax can look a little strange at first.”
started *** playing *** recently *** look…strange *** at first
http://www.jisaacks.com/ruby-tutorial-make-a-tic-tac-toe-game
So, you’re trying to make grow a code over something the guy did
almost 2 years ago when he was LEARNING Ruby.
(I bet he is probably much better now)
The blog is beautiful (graphically) but the code is not as beautiful as
it.
The reason is on his resum. He is pretty good at graphics design (as he
says).
http://www.jisaacks.com/resume
So, it’s difficult to help you go on.
You are looking at a Ruby code without the “good part” of Ruby.
It seems more like a C program “translated” into Ruby.
(Note: I’m not claiming that my code is pretty much different!)
But… just to not disappoint you, I’ll try to do a step-by-step
refactor.
And let’s see if somebody more experienced than me can agree and give
some better advice.
Original code:
def finding_patterns
temp_array = @senarios.map {|element| element == ‘X’ || element ==
‘O’ ? element : " "}
new_array = temp_array.each_slice(3)
times = 0
while times < 12
poss_match = new_array.next.join
@master_hash.each do |key, value|
if key == poss_match
return value,times
end
end
times =+ 1 #!!! this was in
the wrong place and messed up the pattern finding
end
“No finding” #!!!
iterating through @scenarios and @master_hash, it might be better
end #!!! to give the
index counter a name that reflects this
First fall… this kind of iteration using “while” conditionals
with a limit is not usual.
If the array changes size you have to change that “12” limit
We have to use some iterator and with_index
def finding_patterns
temp_array = @senarios.map {|element| element == ‘X’ || element ==
‘O’ ? element : " "}
new_array = temp_array.each_slice(3)
new_array.map.with_index do |ary, ary_ix|
#times = 0
#while times < 12
#poss_match = new_array.next.join
poss_match = ary.join # the “next” array is already at ‘ary’ var
@master_hash.each do |key, value|
if key == poss_match
return value,times
end
end
# times =+ 1 #!!! this was
in the wrong place and messed up the pattern finding
end
“No finding” #!!!
iterating through @scenarios and @master_hash, it might be better
end #!!! to give the
index counter a name that reflects this
Here you’re having the problem because you ‘return’ at the first
finding “breaking” the iteration
What you really want is to ‘select’ ALL the matching itens. Is it
this?
def finding_patterns
temp_array = @senarios.map {|element| element == ‘X’ || element ==
‘O’ ? element : " "}
new_array = temp_array.each_slice(3)
new_array.map.with_index do |ary, ary_ix|
poss_match = ary.join
# @master_hash.each do |key, value|
@master_hash.select do |key, value|
# if key == poss_match
# return value,times
# end
key == poss_match
end
end
“No finding” #!!!
iterating through @scenarios and @master_hash, it might be better
end #!!! to give the
index counter a name that reflects this
But this gives you a Array of 2-elements Array (or an empty array).
And you’re original return value is [value, times]
So you have to “map” these arrays to the proper “format”.
If 'finding_patterns" return an empty Array then it is the "No
finding"
def finding_patterns
temp_array = @senarios.map {|element| element == ‘X’ || element ==
‘O’ ? element : " "}
new_array = temp_array.each_slice(3)
new_array.map.with_index do |ary, ary_ix|
poss_match = ary.join
@master_hash.select do |key, value|
key == poss_match
end.map { |key, value| [value, ary_ix] } # Because ary_ix is the
“new” ‘times’
end.flatten
#“No finding” #!!!
iterating through @scenarios and @master_hash, it might be better
end #!!! to give the
index counter a name that reflects this
For testing, you should just comment the final lines
#print “\n\nYou are X. Please go first.”
#TicTacToeGame.new.game_play
fire up an IRB session and
load “./tic_tac_toe.rb”
=> true
t = TicTacToeGame.new
=> #<TicTacToeGame:0x000000026186c0 @master_hash={“XXX”=>“winner_X”,
“OOO”=>“winner_O”, "XX “=>“last_value_win”, “X X”=>“middle_value_win”,
" XX”=>“first_value_win”, "OO “=>“last_value_block”, “O
O”=>“middle_value_block”, " OO”=>“first_value_block”, "X
“=>“last_two_values_add”, " X “=>“either_end_values_add”, "
X”=>“first_two_values_add”}, @game_board={“11”=>” “, “12”=>” “,
“13”=>” “, “21”=>” “, “22”=>” “, “23”=>” “, “31”=>” “, “32”=>” “,
“33”=>” "}, @senarios=[“11”, “12”, “13”, “21”, “22”, “23”, “31”, “32”,
“33”, “11”, “21”, “31”, “12”, “22”, “32”, “13”, “23”, “33”, “11”,
“22”, “33”, “13”, “22”, “31”], @players={:X=>“X”, :O=>“O”}, @turn=:X>
t.send(:finding_patterns)
t.send(:player_choice) # Choose any line and column
t.send(:finding_patterns) # See the patterns that it finds now
and see the results
Well… done?
Not yet.
Now that we changed the return value of this method we have to change
the code wherever it is used.
For example, the code bellow
#!!! here is the actual code for computer_choice
#!!! the logic was sequential in your code but it needed to be
nested
value, times = finding_patterns # <— This is not possible anymore
location = go_for_the_win(value, times)
if location == nil
location = perform_block(value, times)
if location == nil #!!! debug statement
location = add_to_existing(value, times)
if location == nil
location = random_move
end
end
end
marker_placement_hash(location, ‘O’)
marker_placement_array(location, ‘O’)
At the code above, now you have to iterate over each “pattern”
instead of doing it once.
Do you really want to go on with this?
Brave guy! But…
“The paths of the ruby winsdom to find you need”
“But alone now to go you have”
“Because your friend’s time now short it is”
“And dangerous path your friend thinks you choose”
“Perhaps other day to join you again he will”
Hope it helps you!
Send us any news about the code!
Abinoam Jr.