Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm converting a procedural based site to an OOP design to allow more easily manageable code in the future and so far have created the following structure:

/classes
/templates
index.php

With these classes:

ConnectDB
Games
System
User
User   -Moderator
User   -Administrator

In the index.php file I have code that detects if any $_GET values are posted to determine on which page content to build (it's early so there's only one example and no default):

function __autoload($className) {
    require "classes/".strtolower($className).".class.php";
}

$db = new Connect;
$db->connect();

$user = new User();

if(isset($_GET['gameId']))
{       
    System::buildGame($gameId);
}

This then runs the BuildGame function in the system class which looks like the following and then uses gets in the Game Class to return values, such as $game->getTitle() in the template file template/play.php:

function buildGame($gameId){

    $game = new Game($gameId);
    $game->setRatio(900, 600);

    require 'templates/play.php';
}

I also have .htaccess so that actual game page url works instead of passing the parameters to index.php

Are there any major errors of how I'm setting this up or do I have the general idea of OOP correct?

share|improve this question

migrated from stackoverflow.com Jan 3 '12 at 14:28

This question came from our site for professional and enthusiast programmers.

    
I can't see major nor concept errors. You are doing it well. But let's wait for more expert opinions :) – lorenzo-s Jan 3 '12 at 14:15
    
@Silver89 -- can you put your code up somewhere? (github or the like?) – Neal Jan 3 '12 at 14:17
1  
whether you got the OO parts right or not is impossible to say from the very little code you show. We'd need to see your classes. If you need guidance in refactoring the procedural codebase have a look at sourcemaking.com/refactoring/… – Gordon Jan 3 '12 at 14:21
1  
There isn't enough info in your question, to discuss your approach. You should expand it and give us a better view of the whole hierarchy (not so much code, but how your classes interact with each other) and tell us exactly what you're doing and why. Or you could simply post your classes for review at Code Review Stack Exchange. – Yannis Jan 3 '12 at 19:58
    
You might want to consider implementing an autoloader instead of explicitly requiring everything. – GordonM Jan 4 '12 at 10:12
up vote 11 down vote accepted

As far as OOP goes, there's never really a right or wrong way. There's always more roads that lead to Rome.

However, there are some caveats to avoid. Your System class looks like a God object, which is a so-called 'anti-pattern'. Best to avoid those. Also, I suspect that you're using $db and $user as global variables -- those are not encouraged in OOP. It's best to pass the DB handle to the classes that need it, either upon construction or for instance with a setDatabase() method (this is called Dependency Injection, and makes your classes easier to oversee, maintain and debug).

Finally, an autoloader might make your job as a programmer a little bit easier. This will take the job of doing the require calls out of your hands -- and also forces you to structure your code in a logical manner :)

share|improve this answer
1  
All speculation. I find it pretty hard to judge that System is a God Object. It might just be an Application Factory. Same for the remaining answer parts. You simply cannot know from the OP's code. – Gordon Jan 3 '12 at 14:24
1  
+1 for the autoloader. – Raveline Jan 3 '12 at 14:24
2  
+1 for explaining Dependency Injection – Mark Baker Jan 3 '12 at 17:58

You might take a look at an MVC... you're already splitting your templates and classes for the View and Model/Controller) components, but look at splitting your classes into models and controllers

share|improve this answer
    
Yes I think this is what I'm trying to aim for, quite a few of these terms are quite new to me though so I'm still learning how to go about implementing these solutions. I've just got to the point where I'm beginning to see the real benefits of OOP. – Silver89 Jan 3 '12 at 14:20

You might want to employ an autoloader to load all your classes for you.

You could see how I tried to do it in my (m)vc.

share|improve this answer
    
yes, but the OP asked about OOP and I dont see how this suggestion is on-topic. Should have been a comment. – Gordon Jan 3 '12 at 14:18
    
@Gordon might help the OP organize their code better. – Neal Jan 3 '12 at 14:19

So this is probably too old for me to help the OP in any way, and I'm not an expert. However, I'd like to support @Rijk & their accepted answer with a little bit more structure & a few reference books (as outdated as books are starting to be these are, in my opinion, good ones).

So modern OOP, is mostly centered around the SOLID acronym.
Single responsibility, Open-closed principle, Liskov substitution, Interface segregation & Dependency inversion (almost an exact quote from the link).

They are defined better in the linked article than I could do them justice but here are the overall concepts that most of this fits under (but in , as much as I could pull off, less technical sounding terms):

  • Do one thing & do it well
  • Attempt to keep programming objects smaller & simpler rather than larger & complex (you can still build complex systems with less complex parts and it makes it much easier to understand each part of the complex system... and avoid and fix bugs)
  • Make your classes built so they follow the above AND so you can add functionality to them WITHOUT changing the code that makes up the class (when you change working code you are always risking the creation of bugs)
  • Design to concepts and ideas before you design your code
  • When a concept is implemented in code make an interface for it and anything that fits that concept should support the same interface & that interface to the concept should NOT change
  • Based on the above, make more interfaces & more concepts behind them if the interfaces you have created are not sufficient (rather than make the interface you have defined more complex)
  • Also avoid complex inheritance chains and instead utilize object composition


It's likely I missed a number of important points and/or failed to convey the proper ideas in what I said so, on to the reference books. These books were recommended to me by multiple programmers with more experience, expertise, & knowledge than I have. So with that recommendation:

The second is an excellent book on how to write good software & pull off software projects & plan & design & ... just lots of other valuable stuff.
The first is more accessible in general and is a really great resource for learning, well ... probably all of the principles covered in the SOLID acronym.

The overarching concept seems to be best described as:
writing relatively error free software through writing smaller pieces of code that are proven to work, that also don't need to be changed when you add more functionality.

Hope that's helpful to someone.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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