Multiplayer bowling in Ruby, with variable skill

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 mechanics

• scoring calculations

• stats 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 bud

• proper use of instance/local variables

• use of arrays to store player/game data

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

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 importa

StackExchange Code Review Q#64546, answer score: 8