0

It is also true that a system may become so coupled, where each class is dependent on other classes that depend on other classes, that it is no longer possible to make a change in one place without having a ripple effect and having to make subsequent changes in many places.[1] This is why using an interface or an abstract class can be valuable in any object-oriented software project.

Quote from Wikipedia

Starting from scratch

I'm starting from scratch with a project that I recently finished because I found the code to be too tightly coupled and hard to refactor, even when using MVC. I will be using MVC on my new project aswell but want to try and avoid the pitfalls this time, hopefully with your help.

Project summary

My issue is that I really wish to keep the Controller as clean as possible, but it seems like I can't do this. The basic idea of the program is that the user picks wordlists which is sent to the game engine. It will pick random words from the lists until there are none left.

Problem at hand

My main problem is that the game will have 'modes', and need to check the input in different ways through a method called checkWord(), but exactly where to put this and how to abstract it properly is a challenge to me. I'm new to design patterns, so not sure whether there exist any might fit my problem.

My own attempt at abstraction

Here is what I've gotten so far after hours of 'refactoring' the design plans, and I know it's long, but it's the best I could do to try and give you an overview (Note: As this is the sketch, anything is subject to change, all help and advice is very welcome. Also note the marked coupling points):

Wordlist

class Wordlist {
    // Basic CRUD etc. here!
    // Other sample methods:
    public function wordlistCount($user_id) {} // Returns count of how many wordlists a user has
    public function getAll($user_id) {} // Returns all wordlists of a user
}

Word

class Word {
    // Basic CRUD etc. here!
    // Other sample methods:
    public function wordCount($wordlist_id) {} // Returns count of words in a wordlist
    public function getAll($wordlist_id) {} // Returns all words from a wordlist
    public function getWordInfo($word_id) {} // Returns information about a word
}

Wordpicker

class Wordpicker {
    
    // The class needs to know which words and wordlists to exclude
    protected $_used_words = array();
    protected $_used_wordlists = array();
    
    // Wordlists to pick words from
    protected $_wordlists = array();
    
    /* Public Methods */
    public function setWordlists($wordlists = array()) {}
    public function setUsedWords($used_words = array()) {}
    public function setUsedWordlists($used_wordlists = array()) {}
    
    public function getRandomWord() {} // COUPLING POINT! Will most likely need to communicate with both the Wordlist and Word classes
    
    /* Protected Methods */
    protected function _checkAvailableWordlists() {} // COUPLING POINT! Might need to check if wordlists are deleted etc.
    protected function _checkAvailableWords() {} // COUPLING POINT! Method needs to get all words in a wordlist from the Word class
    
}

Game

class Game {
    
    protected $_session_id; // The ID of a game session which gets stored in the database along with game details
    protected $_game_info = array();
    
    // Game instantiation
    public function __construct($user_id) {
        if (! $this->_session_id = $this->_gameExists($user_id)) {
            // New game
        } else {
            // Resume game
        }
    }
    
    // This is the method I tried to make flexible by using abstract classes etc.
    // Does it even belong in this class at all?
    public function checkWord($answer, $native_word, $translation) {} // This method checks the answer against the native word / translation word, depending on game mode
    
    public function getGameInfo() {} // Returns information about a game session, or creates it if it does not exist
    public function deleteSession($session_id) {} // Deletes a game session from the database
    
    // Methods dealing with game session information
    protected function _gameExists($user_id) {}
    protected function _getProgress($session_id) {}
    protected function _updateProgress($game_info = array()) {}
    
}

The Game

/* CONTROLLER */
/* "Guess the word" page */

// User input
$game_type = $_POST['game_type']; // Chosen with radio buttons etc.
$wordlists = $_POST['wordlists']; // Chosen with checkboxes etc.

// Starts a new game or resumes one from the database
$game = new Game($_SESSION['user_id']);
$game_info = $game->getGameInfo();

// Instantiates a new Wordpicker
$wordpicker = new Wordpicker();

$wordpicker->setWordlists((isset($game_info['wordlists'])) ? $game_info['wordlists'] : $wordlists);
$wordpicker->setUsedWordlists((isset($game_info['used_wordlists'])) ? $game_info['used_wordlists'] : NULL);
$wordpicker->setUsedWords((isset($game_info['used_words'])) ? $game_info['used_words'] : NULL);

// Fetches an available word
if (! $word_id = $wordpicker->getRandomWord()) {
    
    // No more words left - game over!
    $game->deleteSession($game_info['id']);
    redirect();
    
} else {
    
    // Presents word details to the user
    $word = new Word();
    $word_info = $word->getWordInfo($word_id);
    
}

The Bit to Finish

/* CONTROLLER */
/* "Check the answer" page */

// ??????????????????

( http://pastebin.com/cc6MtLTR )

Make sure you toggle the 'Layout Width' to the right for a better view. Thanks in advance.

Questions

  1. To which extent should objects be loosely coupled? If object A needs info from object B, how is it supposed to get this without losing too much cohesion?
  2. As suggested in the comments, models should hold all business logic. However, as objects should be independent, where to glue them together? Should the model contain some sort of "index" or "client" area which connects the dots?

Edit: So basically what I should do for a start is to make a new model which I can more easily call with oneliners such as $model->doAction(); // Lots of code in here which uses classes!

How about the method for checking words? Should it be it's own object? I'm not sure where I should put it as it's pretty much part of the 'game'. But on another hand, I could just leave out the 'abstraction and OOPness' and make it a method of the 'client model' which will be encapsulated from the controller anyway. Very unsure about this.

9
  • 1
    As the question is at the moment, it looks more suited for Code Review, however, since it is not compilable code it is not allowed there. To make the question work better here, and be more than a design review, be sure to add some specific questions you are interested in. Nov 26, 2011 at 15:16
  • I thought about such a site first but had no idea StackExchange had one. I'll keep it in mind for later, thanks!
    – Seralize
    Nov 26, 2011 at 15:28
  • Can you paste the code into the question. Pastebin may not retain the code as long as this question is here. So we need the code on this site to make the questions relevant to the answer. Nov 26, 2011 at 15:41
  • @StevenJeuris: I also think this question would work better on Code Review. I think the question is about reviewing the design of the code so bending the rules is OK in this situation. The spirit of the rule is to prevent people from posting bug fix questions (which belong on stackoverflow). Nov 26, 2011 at 15:44
  • @LokiAstari: Well, the updated question with the two questions at the bottom is very suitable for Programmers in my opinion. If we were to bend the rules and allow not-working code on Code Review the line between Code Review and Programmers wouldn't be that obvious anymore. Nov 26, 2011 at 15:47

3 Answers 3

3

To which extent should objects be loosely coupled? If object A needs info from object B, how is it supposed to get this without losing too much cohesion?

Coupling and cohesion is an important aspect to keep in mind while writing software. However, ideally your code is comprised of high areas of cohesion, loosely coupled to other high areas of cohesion. It shouldn't be a goal to have everything as loosely coupled as possible. It only makes sense to loosely couple those parts of the software when they could be used independently of the other parts.

As suggested in the comments, models should hold all business logic. However, as objects should be independent, where to glue them together? Should the model contain some sort of "index" or "client" area which connects the dots?

Think of all the actions you can do on the model. 'start a game', 'end a game', 'join a game', 'throw dice', ... That are actions which should be exposed in the model. The controller binds the view, where e.g. a button is pressed to start a game, and calls the relevant action in the model.

Regarding your code

At first glance, I'm not sure you separated your entire business logic to your model, and see some of that going on in your controller.

Your model should be able to run on its own separately just by doing calls to it through the controller which simulate user interactions.

4
  • This is exactly what I'm trying to achieve, but no matter how I twist and refactor it I only see the Controller as the solution, because it's the only place where I can "glue the application" together without coupling up the model. This is why I decided to ask for help. Call it the "index/client" if you will, but I have gotten the impression that I should not make a 'Model which connects the dots', that's what the Controller is for in the first place.
    – Seralize
    Nov 26, 2011 at 15:17
  • 1
    Yes, but not the business logic dots. ;p This is the line of code in the controller which worried me: if (! $word_id = $wordpicker->getRandomWord()) And the subsequent action you do then: // No more words left - game over! ... $game->deleteSession($game_info['id']);. That is definitely business logic and should be in your model! Nov 26, 2011 at 15:22
  • Have a look at question #2 and I won't need to repeat myself here :)
    – Seralize
    Nov 26, 2011 at 15:26
  • Great questions in your update! Let me know if my update somewhat answered them. ;p Nov 26, 2011 at 15:45
1

Your game (Model) embodies the idea of behavioral modes. You know that behavior is in the Controller domain, so it seems like your Model drives your Controllers, which can't be right.

But it is in this case. The problem is that you're talking about two levels of abstraction. When your game is in one particular mode, you should have MVC triads -- the Controller gets the user interaction, forwards it to the Model, the View reflects the new Model state. At that level of abstraction, you certainly don't want your Model objects to influence your Controller.

But your problem has a higher-level, in which, at any one time, it is in a mode. The selection of a particular modal set of MVC triads should be under the control of a higher-level Model object (which, itself, is part of a higher-level MVC triad).

I have to admit I don't follow how your particular example, but let's take Soccer, where you have a normal mode and a penalty kicks mode. Conceptually, your higher-level design would go something like:

[PSEUDOCODE]

//High-level model of game
class SoccerModel { 
   Controller activeController; 

   Controller normalController; 
   Controller penaltyKicksController;

   getActiveController()  { return activeController; } 

   chooseGameMode(evt : GameEvent) : Controller { 
         if ( evt == WHATEVER ) { 
              activeController = normalController;
         } else if ( evt == SOMETHING_ELSE ) { 
              activeController = penaltyKicksController;
         }
         return activeController; 
   }
}

interface IController { 
   //Returns an event in the Model domain
   eventForInput(input : SomeType) : GameEvent;
   //Deals with input, presumably by mapping it into a GameEvent and forwarding to Model
   handleInput(input : SomeType) : void;  
}

class GameController : IController {

   SoccerModel highLevelModel; //Terrible name, but is responsible for modeling the game
   ...etc...

   handleInput(myInput) { 
         var gameEvent = eventForInput(myInput);
         var modeController = highLevelModel.chooseGameMode(gameEvent);
         modeController.handleInput(myInput); //Maybe. Allows for more flexibility
   } 

   eventForInput(myInput) { 
         return model.getActiveController().eventForInput(); 
   }
}

The high-level Model object (SoccerModel) holds Controllers, but at a lower-level. The Controllers it holds (penaltyKicksController normalController) do not have circular references back to the SoccerModel.

0

You can take a look at several patterns or elemnts from Domain Driven Design (DDD) that may help you:

A particularly good one is Services:

Services are classes that belongs to the domain layer (let's say your model in this case) and that hold those operations that doesn't belong to any of your entities (Evans, 2004)

So, you may create interfaces for those operations that doesn't belong to your CRUD operation classes and the implementations for such interfaces would be the glue you were looking for.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.