Writing an MVC project for the first time - how close am I?

Question

I've recently started programming (learning Ruby) and wrote a small game. Now I've started reading about MVC and am trying to refracture my code to follow that architecture. The only classes with big changes so far are the intended controller and view (which is completely temporary.

# This file will handle the Demigod class, which is the player controlled class
#
# =============================================================================
require_relative 'UiHandler'
require_relative 'GameData'
require_relative 'World'
class Demigod
 attr_reader :resources
 @world
 # Starts a new game - each demigod has one world to rule
 # accepts different starting resources as a parameter
 def initialize (world, resources = GameData::STARTING_RESOURCES)
 @world = world
 @resources = resources
 end
 # Handles a turn, main function of the game
 def turn
 # calculates new amount of resources using the production function
 # world.production accepts a resources hash and returns a newly
 # calculated resource hash based on production
 @resources = @world.production(@resources)
 p @resources
 #do_event # ========= TODO ========= #
 # part 2 of every turn
 UiHandler.print_world(@world)
 UiHandler.print_turn_message
 # Asks the user for a tile to do his action on
 decision = UiHandler.get_decision()
 until (@world.valid?(decision) || decision == '')
 UiHandler.print_error(UiHandler::NO_TILE)
 UiHandler.print_turn_message
 decision = UiHandler.get_decision()
 end
 unless decision == ''
 tile = @world.get_tile decision # returns the tile at decision
 # checks for legality of move on tiles using tile.accepts? and tile.check_cost
 until tile.accepts?(decision)
 break if decision == ''
 UiHandler.print_tile_options(tile)
 decision = UiHandler.get_decision()
 if (!tile.accepts?(decision))
 UiHandler.print_error(UiHandler::INVALID)
 elsif (!tile.check_cost(decision, @resources))
 decision = nil
 UiHandler.print_error(UiHandler::RESOURCES)
 end
 end
 if decision != ''
 price = GameData.get_price(decision)
 @resources = reduce(price)
 @world.advance(tile, decision)
 end
 end
 # Clears the screen
 UiHandler.clear_messages()
 end
private
 # calls an new event # === TODO === #
 def do_event
 end
 # Reduces resources TOBEMOVED to world
 def reduce(amount)
 amount.each do |name, value|
 @resources[name] -= value
 end
 @resources
 end
end

Stuff I realized while writing this question:

• resources really shouldn't be here, though I'm not sure if they deserve their own class or should just be in the world (the main model).
• I think that the loop dealing with incorrect input should be part of the view, since it interacts directly with the user for input, but I'm not sure how to do it.

I would appreciate a review of how the entire code fits MVC, but I don't expect it.

Answer 1

This is a little abstract, without much actual code, but perhaps it's useful nonetheless.

A game is actually a little tough to apply "stringent" MVC to. It's not quite clear cut what belongs where. The world is obviously a model, resources are obviously models, and tiles are too. But is a player a controller or a model? That's more difficult.

I'd say that you shouldn't worry about what the player is. Instead you should add an overall "game controller". Its responsibility would be to update resources before and after #turn is called on a player. The game controller would also be responsible for printing the world, etc., while the player would only responsible for actually making a turn (or, given that there seems to be 2 distinct parts to a turn, make those 2 separate methods). Return a decision (or two), and let the game controller and world sort out how to change the game state (reduce resources, update tiles, etc.).

You might also wait until this point to check the validity of a decision. Let the player make a decision first, and if it's nonsense, let the player try again. The player doesn't necessarily have to know the rules of the game, as long as the game controller knows 'em.

Never mind if the player is M, V, or C. Approach it simply as a "decision maker". It may be AI or a human player, but the point is that it returns a decision. It's sort of a controller, but I'd avoid having it update models or updating the view. It's basically a service object. It could even be completely stateless: Give it a world and a set of resources, and let it come up with a decision somehow.

---

In terms of code review:

My gut reaction is that there's too much going on in #turn. This is irrespective of the MVC design; the method is just doing too much. (In Ruby you often aim for methods that are super, super short. If you go above 5 lines, you need a reason. Above 10 lines, and you need a very good reason).

Many, if not all, of the UIHandler methods might work better as a module. They appear state-less, so it might well be simpler to just include them where necessary.

There are also some mixed responsibilities. You have tile.check_cost(decision, @resources) but you also have price = GameData.get_price(decision). If you can get the price yourself, you might as well check the cost yourself too. The check_cost method is nicely abstract - you don't need to worry about the cost, just if you can afford it. But that's undermined by you having to fetch the price later anyway.

On a micro level: To get a decision from the command line, I'd recommend something like

def get_decision(&block)
 begin
 input = gets.chomp # or however you read input
 end until !block_given? || yield(input)
 input
end

which you then call like

decision = get_decision { |input| input.empty? || possible?(input) }

where possible? would be a method that checks tile.accepts?(input) and whether it fits the available resources.

Most of all, since there seems to be two steps in a "turn", I'd split those into separate methods. Again, have a game controller and let it call one method first, then the other.