This is a multiplayer bowling simulator in Ruby. It allows for variable skill levels, and produces weighted random results for each roll based on those skill settings. The methods are grouped in four sections: game mechanicsscoring calculationsstats calculators (because I got curious about what my skill/weighting system was generating)gameplay options.The game mechanics fill a nested array with the results of individual rolls. The scoring operates on those to fill a second array with the frame-by-frame scores. The screen outputs are scattered through the various methods, but they get turned off when you run multiple games to check stats. Everything works correctly.  The code is also here.I'm aware of the more straightforward 'kata'/Robert Martin version of the bowling problem (where you do all the rolls first and only then score the complete game). But I first encountered the problem in this form -- a game that's scored in progress, like an actual bowling alley display -- and wanted to solve it this way, along with the additional challenges noted above. I'm new to Ruby and to coding, and experimenting on my own. I haven't figured out 'unit tests' yet so there aren't any. I also suspect this could/should all be wrapped in a single class, but haven't had a chance to work that out. If that's necessary to make the code reviewable, please let me know.I'd appreciate critiques/advice on any of the following:obvious rookie mistakes or bad habits to nip in the budproper use of instance/local variablesuse of arrays to store player/game datadefinition and length of methods, relative to either design logic or general readability(Some I couldn't keep under 10 lines, given the number of cases/conditionals to sort through, but I tried to follow the short-method recommendation as much as possible. The scoring methods definitely need improvement, but I'm interested in where else additional refactoring is warranted.) order of methods within code(I know it doesn't affect execution, but how much does it matter for readability?)The system I'm using for weighted rolls (line 120 in roll_calc)(I considered rand(0..pins_remaining + skill), then rounding down anything over the number of pins; it's low-cost and makes it easy to increase the number of strikes, but otherwise an 8 is as likely as a 2 -- not a good representation. I also considered taking the nth-root of rand(o..pins_remaining**n), which makes higher numbers proportionally more likely for a better bowler, in a nice way, but seemed really costly.)Where I'm furthest off the mark in terms of Ruby style/standardsGame mechanics:# ===============================================# GAME MECHANICS#def newgame  @players = Array.new  @frame_scores = Array.new  @game_scores = Array.new  @player = 1  @frame_no = 1  @roll_type = 1  @pins_remaining = 10enddef get_players  print How many players(1-4)?      numplayers = gets.chomp.to_i    numplayers = 1 if numplayers < 1    numplayers = 4 if numplayers > 4  get_skill(numplayers)  reset_arrays  print_playersenddef get_skill(numplayers)  puts Enter skill level 0-15 (2+ = good, 4+ = v.good, 6+ = pro):  (1..numplayers).each do |p|    print Skill level for player #{p}?      skill = gets.chomp.to_i    skill = 0 if skill < 0    @players << skill  endend# prepares nested arrays to be filled during gameplay/scoring#def reset_arrays  @players.length.times do      @frame_scores << []      @game_scores << []      endenddef print_players  puts  (1..@players.length).each do |p|    print P#{p}(#{@players[p - 1]}).rjust(7), ___________   end  putsenddef playgame  until @frame_no == 11    roll_control(@player)     end  puts unless @stats_mode == trueenddef turn_control(player)  if player == @players.length    puts unless @stats_mode == true    @frame_no += 1    @player = 1  else    @player += 1  end  @roll_type = 1   @pins_remaining = 10 enddef roll_control(player)  if @roll_type == 4                      # for 10th/3rd    roll_calc(player,1)    score_progress(player,@frame_no - 1)     turn_control(player)      elsif @roll_type == 3               # 1st bonus after 10th fr strike -> to 3rd roll    roll_calc(player,1)    @pins_remaining = 10 if @pins_remaining == 0    @roll_type = 4                else    roll_calc(player,@roll_type - 1)            if @roll_type == 1          # regular first roll      if @pins_remaining == 0 and @frame_no == 10        @roll_type = 3                                                   @pins_remaining = 10      elsif @pins_remaining == 0        score_progress(player,@frame_no - 1)        turn_control(player)      else        @roll_type = 2                end        elsif @roll_type == 2       # regular second roll          if @pins_remaining == 0 and @frame_no == 10            @roll_type = 4                                                   @pins_remaining = 10      else        score_progress(player,@frame_no - 1)        turn_control(player)      end    end      end  enddef roll_calc(player,type)  skill = @players[player - 1]  picks = Array.new  (skill + 1).times do     pins = rand(0..@pins_remaining)    picks << pins    break if pins == @pins_remaining  end  pins_hit = picks.max    if type == 0     @frame_scores[player - 1] << [pins_hit]  else     @frame_scores[player - 1][@frame_no - 1] << pins_hit  end  @pins_remaining = @pins_remaining - pins_hitendScoring methods:# ===============================================# SCORING METHODS# i in next 3 methods is array index of current frame (so @frame_no - 1)#   should be more descriptive, but lines are already too long --#   better to create variables for array values being summed, then add? #def score_progress(player,i)  framesum = @frame_scores[player - 1][i].reduce(:+)  if i >= 2 && @game_scores[player - 1][-2].nil? == true    update_2prev(player,i)  end  if i >= 1 && @game_scores[player - 1][-1].nil? == true    update_prev(player,i,framesum)  end  if i == 9     @game_scores[player - 1] << @game_scores[player - 1][-1] + framesum  elsif framesum == 10    @game_scores[player - 1] << nil  elsif i == 0    @game_scores[player - 1] << framesum  else     @game_scores[player - 1] << @game_scores[player - 1][i - 1] + framesum  end  unless @stats_mode == true    print @game_scores[player - 1][i].to_s.rjust(6)    print  , @frame_scores[player - 1][i].inspect.ljust(12)  endenddef update_2prev(player,i)  if i == 2    @game_scores[player - 1][-2] = 20 + @frame_scores[player - 1][i][0]  else    @game_scores[player - 1][-2] = @game_scores[player - 1][-3] + 20 + @frame_scores[player - 1][i][0]  end  enddef update_prev(player,i,framesum)  if @frame_scores[player - 1][i - 1][0] == 10 && @frame_scores[player - 1][i][0] == 10      if i == 9        @game_scores[player - 1][-1] = @game_scores[player - 1][-2] + 10 + @frame_scores[player - 1][i][0] + @frame_scores[player - 1][i][1]      end    return                                 elsif i == 1     if @frame_scores[player - 1][0][0] == 10      @game_scores[player - 1][0] = 10 + framesum    else      @game_scores[player - 1][0] = 10 + @frame_scores[player - 1][1][0]    end  else    if @frame_scores[player - 1][i - 1][0] == 10        if i == 9           @game_scores[player - 1][-1] = @game_scores[player - 1][-2] + 10 + @frame_scores[player - 1][i][0] + @frame_scores[player - 1][i][1]              else          @game_scores[player - 1][-1] = @game_scores[player - 1][-2] + 10 + framesum        end    else      @game_scores[player - 1][-1] = @game_scores[player - 1][-2] + 10 + @frame_scores[player - 1][i][0]    end  endendStats tests:# ===============================================# STATS TESTS## average score by skill setting#def average_test(limit)  (0..15).each do |skill|    gamecount = 0    pintotal = 0    minscore = 300    maxscore = 0    until gamecount == limit      newgame      @players << skill      reset_arrays      playgame      gamecount += 1      pintotal += @game_scores[0][-1]      minscore = @game_scores[0][-1] if @game_scores[0][-1] < minscore      maxscore = @game_scores[0][-1] if @game_scores[0][-1] > maxscore    end    print #{skill}\t#{gamecount}\tavg    print #{(pintotal / gamecount)}.rjust(4), #{minscore}/#{maxscore}.rjust(9), \n    puts  endend# frequency of perfect game by skill setting#def perfect_test(limit)  (0..15).each do |skill|    gamecount = 0    perfect_games = 0     until gamecount == limit      newgame      @players << skill      reset_arrays      playgame      gamecount +=1            perfect_games += 1 if @game_scores[0][-1] == 300      end    print skill, \t, gamecount, \t, perfect_games, \n  endend# head-to-head wins for consecutive skill levels# (n.b. diff greatest at low skill - approach 50/50 at high)#def upset_test(limit = 1)  puts  (0..14).each do |skill|    wins = [0,0,0]    gamecount = 0    until gamecount == limit      newgame      @players << skill      @players << (skill + 1)      reset_arrays      playgame      wins[0] += 1 if @game_scores[0][-1] > @game_scores[1][-1]      wins[1] += 1 if @game_scores[0][-1] < @game_scores[1][-1]      wins[2] += 1 if @game_scores[0][-1] == @game_scores[1][-1]      gamecount += 1    end    print @players.inspect.rjust(8),    , wins.inspect, \n  endendPlay options:# ===============================================# PLAY MODES:#def stats_tests(limit)  @stats_mode = true    # use to disable gameplay screen output  puts  average_test(limit)  perfect_test(limit)  upset_test(limit)  enddef repeat_check  puts \nRepeat game with these players (y/n/stats/quit)?  repeat = gets.chomp.downcase  if repeat == stats    print Number of games per test?      limit = gets.chomp.to_i    stats_tests(limit)  elsif repeat == n    single_game  elsif repeat == y    curr_players = @players    newgame    @players = curr_players    reset_arrays    print_players    playgame    repeat_check  else    return  end  enddef single_game  newgame  get_players  playgame  repeat_checkendsingle_game
Multiplayer bowling in Ruby, with variable skill
beginner;ruby;game
Nice question! I may have a go at writing an implementation myself and put it up for review; it's a fun challenge.Review-wise I haven't gone into great detail (there's a lot of code there), but here are some thoughts:Encapsulation. Right now, everything is happening in the global object. It's just one big soup of methods and data. Ruby is object-oriented, so you should create classes to encapsulate related data and methods. For instance, I might start by creating a Player class, which would model a single player (obviously). This class might also be responsible for tracking that player's skill level, score, frames bowled, and stuff like that (or perhaps that should be in a separate class?). A more high-level Game class could handle several players, and thus model the game itself, the order of players, and figure out the winner by examining the players' scores.This is just off the top of my head; another structure might be more appropriate. For instance, the Game class could just keep track of the frames, and the each frame could, in turn, reference the player that played it.It's all a question of how you describe (model) the game of bowling. Is it a game where players play frames or a game where frames are played by players? Maybe both? Domain modelling is hard, but plan a little, code a little, and see what works. Optimally using tests/specs.Separation of concerns. As you say the screen outputs are scattered through the various methods. This is a red flag. For one, it's not DRY to repeat code (such as the code prompting the user for something) all other the place. For another, your classes (which I'll just assume you've created since reading the above), are ideally abstract clumps of data and logic. They shouldn't also be responsible for user interaction. Rather, you should separate the logic that specializes in user interaction, from the logic that does the scoring, from the logic that handles skill level, etc..Coupling. Right now, in part because stuff isn't encapsulated, you have a tangled web of dependencies. For instance, you have stuff like update_prev(player,i,framesum), which takes 3 arguments (and which could use some extra whitespace to breathe, but that's another story). That'd be fine if all the method did was work only on those three arguments. But 2 of those arguments are each indirect references to something else, since they're just array indices. So you jot down some index from the @players array and pass it to the method. Now, the method also has to know about, and access, the @players array. Both ends of the system are dependent on (coupled to) that array, but neither one is really responsible for it. It just sort of has to be there. It's a game of Jenga, where all the parts interlock, so the slightest touch will knock the whole thing over.Generally, if you've encapsulated stuff well, the various parts of the code know as little as possible about the rest of the system. For instance, a Player object doesn't know about prompting the user; it's just told its skill level. A Game object may not know anything about skill levels or frames, it just knows it's got, say, 3 player objects. And perhaps you have a Frame class, which doesn't know much about anything - it just stores (and perhaps generates) 2 numbers and a total. That's decoupling in a nutshell. Compartmentalization would be another word to describe it and encapsulation.Top-down design. This is more a way of approaching the code. Basically, it's easy to think ok, I know that at some point I have to add up the number of pins knocked over - and then start by coding that part and getting stuck in the minutiae and implementation details. It's easy because you get that part. That part you can solve now. But you quickly lose sight of what you're actually trying to answer: Who won the game? This is bottom-up design, and it's rarely the best way to do things. So, instead, try starting out by thinking this is how I'd like to do X. For instance, in a bowling sim, I'd like to be able to say stuff like some_player.score and game.finished?. With building blocks like those, it's easy to figure out which player won the game. But of course it's all imaginary, since you haven't written the code yet. But you just pick one of those, and repeat the process. E.g. to figure out a player's score, it'd be nice to be able to say frame.score? for each frame and stuff like that. And then you delve into those methods and imagine the ideal way to write them. You continue until you hit bedrock; the level at which you're working with such a simple problem that you don't need more intermediary building blocks to help you. It's really not that different from writing an outline for a paper, before you start writing the content. It takes practice, and sometimes you code yourself into a corner, but eventually it'll come naturally. It's not that you have to slavishly follow this process, it's just important to stay cognizant of the big picture.Alright, so that was the high-level analysis. Again, I didn't go through this line-by-line, but I plucked a few things I noticed. Note, though, that to really get somewhere, you'd have to take all the above into consideration first. Sure, you can make small tweaks, but it's (pardon the harsh phrase) lipstick on a pig.@frame_scores[player - 1]... you repeat this again and again. At least make a local variable like player_index = player - 1, and use that instead.A whole lot of inscrutable conditionals. I've already mentioned it, but something likeif @frame_scores[player - 1][i - 1][0] == 10 ...makes be shiver. What about at least having a constant for a strike?if @frame_scores[player - 1][i - 1][0] == STRIKE ...or better yet, if you model a Frame as a class, you can give it a strike? method:if @frame_scores[player - 1][i - 1][0].strike? ...Ultimately, what I'd like to see is something likeif player.last_frame.strike? ...I could find more micro-level stuff, but again, the overall structure is the biggest bugbear here.By the way, I highly recommend Sandi Metz's Practical Object-Oriented Design in Ruby as a great book for learning about all of this stuff.And do look into unit testing. It's not just a skill, it's a habit - a good habit - to pick up. So pick it up early.