OOP PHP with language detection

Question

I cannot quite see all the advantages of OOP PHP. I have already started translating some procedural functions into object oriented. How is the second code better than the first one? Is there anything I am missing? Could it be improved? It is surely a newbie question but I'm starting now with the Object Oriented aspect. Thank you all!

Old Procedural code

//LANGUAGES
//Detect the language
if ( !empty($_POST['lang']) )    // If the user tries to change the language
  {
  $Lang = mysql_real_escape_string($_POST['lang']);  // Assign the language to variable $Lang.
  $_SESSION['lang']= $_POST['lang'];      //Sets the session to have that language.
  if ($Logged==1)          // If user is logged
    mysql_query("INSERT INTO users (lang) VALUES ('$Lang') WHERE user='$User'");  // Saves the language into user preferences
  }
else      // If no request is done.
  {
  if ( !empty ($_SESSION['lang']))  // If the session exists (not empty)
    $Lang = $_SESSION['lang'];  // Assign the session language to variable $Lang.
  else          // If it doesn't exist
    $Lang = substr ($_SERVER['HTTP_ACCEPT_LANGUAGE'], 0, 2);  // Get it from the browser
  }

//If the language is not supported (or still doesn't exist), then put "en" as default. Supported so far: en, es.
if ( $Lang !== "en" && $Lang !== "es") $Lang="en";

////lang() function
  //Return the right name of the language.
  //Taken from here: http://www.w3schools.com/tags/ref_language_codes.asp
  //And here: http://web.forret.com/tools/html.asp
function lang ()
  {
  global $Lang;

  // Action to do.
  $numargs = func_num_args();  // Get the number of arguments that are being passed.
  if ($numargs == 1)  // If there is one
    $Arg=func_get_arg(0);  // Set $Arg with that value. Example: $ArgLang="en"
  else return $Lang;  //If there is no language passed, echoes the default one

  switch ($Arg)
    {
    case "bn": echo "বাংলা"; break;    //Bengali
    case "de": echo "Deutsch"; break;           //German [Deutsch]
    case "en": echo "English"; break;           //English
    case "es": echo "Español"; break;      //Spanish [Español]
    case "fr": echo "Français"; break;     //French [Français]
    case "it": echo "Italiano"; break;          //Italian [Italiano]
    case "ja": echo "日本語"; break;    //Japanese
    case "pt": echo "Português"; break;    //Portuguese [Português]
    case "ru": echo "русский"; break;    //Russian [русский]
    case "zh": echo "简体中文"; break;    //Simplified chinese [简体中文]
    default: echo "Weird error. Please report."; break;
    }
  }

New Object Oriented code

class Language
  {
  public $Lang;

  public function __construct()  
    {
    if ( !empty($_POST['lang']) )    // If the user tries to change the language
      {
      $this->Lang = mysql_real_escape_string($_POST['lang']);  // Assign the language to variable $Lang.
      $_SESSION['lang']= $_POST['lang'];      //Sets the session to have that language.
      if ($Logged==1)          // If user is logged
        mysql_query("INSERT INTO users (lang) VALUES ('$this->Lang') WHERE user='$User'");  // Saves the language into user preferences
      }
    else      // If no request is done.
      {
      if ( !empty ($_SESSION['lang']))  // If the session exists (not empty)
        $this->Lang = $_SESSION['lang'];  // Assign the session language to variable $Lang.
      else          // If it doesn't exist
        $this->Lang = substr ($_SERVER['HTTP_ACCEPT_LANGUAGE'], 0, 2);  // Get it from the browser
    //If the language is not supported (or still doesn't exist), then put "en" as default. Supported so far: en, es.
    if ( $this->Lang !== "en" && $this->Lang !== "es") $this->Lang="en";
    }

  //Return the right name of the language.
  //Taken from here: http://www.w3schools.com/tags/ref_language_codes.asp  and  here: http://web.forret.com/tools/html.asp
  public function lang($Arg)
    {
    switch ($Arg)
      {
      case "bn": return "বাংলা"; break;    //Bengali [বাংলা]
      case "de": return "Deutsch"; break;           //German [Deutsch]
      case "en": return "English"; break;           //English
      case "es": return "Español"; break;      //Spanish [Español]
      case "fr": return "Français"; break;     //French [Français]
      case "it": return "Italiano"; break;          //Italian [Italiano]
      case "ja": return "日本語"; break;    //Japanese
      case "pt": return "Português"; break;    //Portuguese [Português]
      case "ru": return "русский"; break;    //Russian [русский]
      case "zh": return "简体中文"; break;    //Simplified chinese [简体中文]
      default: return "English"; error("Invalid language"); break;           //English + error
      }
    }
  }

Answer 1

When you have just 1 class, it doesn't really come into perspective...the fact that you're modularizing your code is one of the many bonuses of OOP. Here's your code, cleaned up, with an added Database class:

class DB{
    private $query; 

    public function __construct(){}

    /**
     * Save a user preference
     */
    public function SaveUserPreference($table, $values, $where){
        $this->dbConnect();

        # Build the query
        $this->query = "INSERT INTO ";
        if( !empty($table)  )  $this->query .= $table;
        if( !empty($values) )  $this->query .= "VALUES(" . $values . ") ";
        if( !empty($where)  )  $this->query .= "WHERE " . $where; 

        mysql_query($this->query);
    }

    /**
     * Clean data using mysql_real_escape_string
     */
    public function cleanData($data){
        return mysql_real_escape_string($data);
    }

    /**
     * Create a mysqli connection
     */
    private function dbConnect(){}
}

And here's your Language class, which extends class DB, so that it can use its methods:

class Language extends DB{
    public $language, $isLogged;

    public function __construct(){
        parent::__construct(); 
    }

    public function setPost($_POST){
        $this->language = $this->cleanData($_POST);
    }

   /**
    * If the user tries to change the language
    */
    function setLanguage(){
        if( !empty($this->language['lang']) ){
            $_SESSION['lang'] = $this->language['lang'];

            # If logged, save the language into user preferences in db
            if ($this->isLogged == 1)
                $this->SaveUserPreference("users(`lang`)", $this->language, "user=".$User);
        } else {
            if ( !empty ($_SESSION['lang']) )  # If the session exists
                $this->language = $_SESSION['lang'];  # Assign the session language to variable $Lang.
            else  # If it doesn't exist
                $this->language = substr($_SERVER['HTTP_ACCEPT_LANGUAGE'], 0, 2);  # Get it from the browser

            # If the language is not supported (or still doesn't exist), then put "en" as default. Supported so far: en, es.
            if ( $this->language !== "en" && $this->language !== "es") $this->language = "en";
        }
    }

   /**
    * Return the right name of the language.
    * @Ref: http://www.w3schools.com/tags/ref_language_codes.asp
    * @Ref: http://web.forret.com/tools/html.asp
    */
    public function GetLanguage($arg){
       switch ($arg){
            case "bn": return "বাংলা"; break; # Bengali
            case "de": return "Deutsch"; break; # German
            case "en": return "English"; break; # English
            case "es": return "Español"; break; # Spanish [Español]
            case "fr": return "Français"; break; # French [Français]
            case "it": return "Italiano"; break; # Italian [Italiano]
            case "ja": return "日本語"; break; # Japanese
            case "pt": return "Português"; break; # Portuguese [Português]
            case "ru": return "русский"; break; # Russian [???????]
            case "zh": return "简体中文"; break; # Simplified chinese [????]
            default: return "English"; break; # return English
        }
    }
}

Good, now lets use our classes. Note that we only initialize the parent class, and we don't need to init the DB class separately because we can call the DB methods from the parent class.

#Initialize our class
$r = new Language;

# Build a fake post for testing
$_POST = array('lang' => 'en', 'asa' => 54);

# Set options
$r->setPost($_POST);
$r->isLogged = 1;

# Set Language
$r->setLanguage();

Now that we've saved our users language preferences, let's see what the long form is:

# Now, get the long version of the user language
echo $r->GetLanguage($r->language);

That was easy! And no need to write a mysql_query here and there, just write once and forget about it. We have a set of classes that is modular, abstract, and the parent class inherits methods from its child classes, so no need to reinitialize a DB class, we can just recycle the language class (although in many cases, you shouldn't do this).

OOP is good in some cases, not all. You wouldn't bring a gun to a tickle-fight, would you? Your use of OOP completely depends on what your project is and how big it is, amongst other things.

By the way, here's the above code in action: http://juanleonardosanchez.com/data/sandbox/php/test.php

You can give it different codes via GET: http://juanleonardosanchez.com/data/sandbox/php/test.php?lang=fr

Answer 2

I'm glad to see you lost the globals. That's the first thing I saw and I immediately had to perform an exorcism on my computer. Globals are always bad, avoid them like the plague. I have a few comments to add to jsanc623's post. He had a good start, but there are some things he did not cover, or that he could do better as well.

First, in your attempt you escaped the POST variable very nicely for MySQL, but then you dumped the unmodified version into the SESSION to be reused. It will be clean the first time this is run, but then every other time it will be using raw data. This is bad, just pass the session the class property $Lang since it has already been cleaned.

$this->Lang = mysql_real_escape_string( $_POST[ 'lang' ] );
$_SESSION[ 'lang' ] = $this->Lang;

It appears that you are recreating a boolean with your $Logged variable. Don't do this, just set it to a boolean so you don't have to worry about type conversions, then you can do cool things like below. Admittedly, you can do this without booleans, but still, the concept is sound, no reason to reinvent a language construct for such a simple task.

if( $Logged ) {
//OR
if( ! $Logged ) {

A switch statement in this context seems inefficient. I would set this up in an array like so:

$languages = array(
    "bn" => "বাংলা",//Bengali [?????]
    "de" => "Deutsch",//German [Deutsch]
    //etc...
);

$language = array_key_exists( $Arg, $languages ) ? $languages[ $Arg ] : 'English';

Here's something from jsanc623's post. If a variable is empty, appending it onto a string will not change that string, so all these empty() checks are unnecessary and just add overhead. Additionally, both of you appear to be fond of braceless if statements. This is PHP not Python. While this is possible, it is not advisable. This can lead to mistakes and illegible code. Normally I rant about PHP also considering this bad practice, but they have since removed it and have updated their "if" doc page to reflect this syntax. I think this was a bad move as it misleads beginners because it doesn't explain that this syntax only allows one argument. Some helpful commenter mentioned it, but PHP neglected to.

if( !empty($table)  )  $this->query .= $table;
if( !empty($values) )  $this->query .= "VALUES(" . $values . ") ";
if( !empty($where)  )  $this->query .= "WHERE " . $where; 

When extending a class, it is not necessary to redefine the constructor unless you are planning on manipulating it. I believe this is a habit Java people are bringing over to PHP. Its only necessary to override and then call the parent constructor if you are planning on changing some data going into the parent constructor, or that coming out.

public function __construct(){
    $before = 'variable used by parent constructor, usually passed as param';
    parent::__construct(); 
    $after = 'variable defined in parent constructor';
}

I agree with the rest of jsanc623's post. As he said, OOP only really starts to make sense when used between many classes. Its not always necessary to use classes, sometimes it makes more sense to use just plain procedural code and functions. Just remember, classes/functions don't define OOP/Not-OOP. A class that doesn't follow the key principles of OOP is just a class. And while I've not really looked into it, you can apparently have groups of functions that are considered OOP too.

UPDATE

Perhaps I jumped to conclusions on jsanc623's post. My only excuse is that I mostly skimmed over it, I should really stop that. It does give the general "right" idea. But, just as Corbin said, there are some major problems in it.

Language should not extend DB because it isn't a database. It can use the db, but extending it is the wrong thing to do here. Extending is what you do when two classes implicitly share resources and a purposes. For instance, you could create a MySQL class to extend DB, not saying to do that, just trying to demonstrate. In this instance, you would create a db property in the Language constructor so that it can be reused through out the rest of the class.

public function __construct() {
    $this->db = new DB();
}

I believe the empty methods in jsanc623's classes are there to illustrate the need for them, not to demonstrate using empty methods. You should still initialize your class with a constructor. I don't know what kinds of things you would need to do to initialize your object. There are some generic things I can think of off the top of my head, such as setting default values to class properties and calling the methods necessary for creating a database or connecting to one, but the exact things you will need to do are up to you. Same with the dbConnect() method.

cleanData() is not redundant. The class doesn't know that it needs to clean data for MySQL, this method could just as easily be changed to use htmlspecialchars, or anything else. This is one of the core ideas behind OOP. You have a method whose current contents can be changed without disrupting the flow of the rest of the program. A class should be generic enough to be used in many situations, not specialized for one only. That's what extending classes is for, and even then the specializing should only go so far as to add features and not change it so that other situations couldn't reuse the same solution. Think of that MySQL extending DB scenario again. They are both databases and share very similar characteristics. MySQL still does everything DB does, but it is specialized for MySQL and it can be reused in other situations that require MySQL.

I hope this makes sense, I feel like I'm starting to ramble. OOP is one of the harder concepts to grasp. You could spend days looking at descriptions and explanations before one, with only slightly different wording, finally clicks for you. At least, that's how it was for me. Don't be discouraged, keep asking questions.

Oh, and before I forget. Passing $_POST as a parameter in a function/method, is a bad idea. I would say even passing it TO a function/method is bad. This is a reserved PHP super global, as such, it contents should not be changed only used. Make a copy of it and manipulate that.