tags:

views:

262

answers:

4

Hi,

For long time reading and testing, but i want know. This is correct PHP OOP code, or not

Class User {

  function Add($Name, $Password){
    $sql_str = "INSERT INTO User SET Name = '$Name', Password = '$Password'";
    $sql->do_sql($sql_str);
  }

  function Del($UserID) {
    $sql_str = "DELETE FROM User WHERE UserID = '$UserID'";
    $sql->do_sql($sql_str);
  }

  function Show ($Limit)
    if ($limit > 0){
      $sql_str = "SELECT * FROM User ORDER BY Name LIMIT $Limit";
    }else{
      $sql_str = "SELECT * FROM User ORDER BY Name";
    }
    $result = $sql->do_sql($sql_str);
    for ($i = 0; $i < COUNT($result); $i++){
      $data[$i]['UserID'] = ....
      $data[$i]['Name'] = ....
    }
    return $Data
  }

}
$MyUser = new User;

And now from the file userControl.php I can control the actions. If I want to do something, I can send the action to the instance of the user class: $MyUser->Add($Name, $Password); Is this approach more like a grouped function and not OOP or is it better to use setters and getters?

If this example not OOP, then what I do wrong and how need to do this example OOP way?

Tnx

A: 

Technically it is, but you're either missing a lot of code or your methods won't work. You don't seem to define $sql anywhere. Since the beauty of oop really shines when eliminating duplicate code, and you use $sql in all of your methods, it would be nice to see how you dealt with that. Without complete, working, code it's hard to provide suggestions.

Here is a brief example of what I mean. Since you aren't using any oop features of PHP5 I'll stick with PHP4:

class User
{
  var $sql;
  function User()
  {
    $this->sql = new DatabaseConnection();
  }

  function add($data)
  {
    $query = '...query here...';
    $this->sql->query($query);
  }
}

If you want to check out some examples of solid, enterprise-level code, I would highly recommend looking at some of the components in Zend Framework.

Mike B
+4  A: 

You're not going about this the right way. What you really want to do is have a class User that represents a single user, with methods reflecting this.

From wikipedia:

In object-oriented programming, a method is a subroutine that is exclusively associated either with a class (in which case it is called a class method or a static method) or with an object (in which case it is an instance method).

A user object should at very least have instance methods enabling it to:

  • Load from the database
  • Save to the database

And a static method to: - Create a user and return a user object.

It should also have a constructor method (__construct(args) in PHP5 or User(args) in PHP4) to be called when the user is created. This should probably accept an id or a username or something identifying so it can load up the right user.

For the sake of simplicity and not just doing everything for you, imagine a user object with just an id and a name. Here's how the class might look:

Assuming PHP5:

class User{
    private $id;
    public $name;

    public function __construct($id){
        $this->load($id);
    }

    public function load($id){
        // Do a query to load a user and initialize $id and $name.
    }

    public function save(){
        // Do a query saving $this->id and $this->name to the database.
    }

    public static function create($name){
        // Do a query to create a user with name $name.
    }
}

You can load a user given his id using new User($id), or create one given User::create($name);

At risk of being figuratively crucified, I wouldn't bother with setters and getters in PHP.

Adam Bard
+1  A: 

$MyUser->Add($Name, $Password); looks weird. Try something like this:

class UserManager {
    public function add(User $user) {
        $sql->do_sql("INSERT INTO users (id, name) VALUES (".$user->getId().", ".$user->getName().")");
    }
    public function delete(User $user) {
        $sql->do_sql("DELETE FROM users WHERE id = ".$user->getId()." LIMIT 1");
    }
    public function show(User $user) {
        return $sql->do_sql("SELECT * FROM users WHERE id = ".$user->getId());
    }
}

and

class User {
    private $_id;
    private $_name;
    public function getId(){
        return $this->_id;
    }
    public function getName(){
        return $this->_name;
    }
}

A design pattern that might fit is Active Record.

chelmertz
A: 

Thanx! I know some thing about OOP are not correct place my mind, I need shake. Why I do things like I do. First I use template engine. After user post data then this data post action file. There somthing this actionUser.php :

$op = '';
IF (ISSET($_REQUEST['op'])){
  $op   = ADDSLASHES($_REQUEST['op']);
}

if ($op == 'AddUser'){
 $Name = ADDSLASHES($_REQUEST['Name'])
 $Password = ADDSLASHES($_REQUEST['Password'])
$MyUser->Add($Name, $Password)
}

Then send action to class user.

User Class have litle bit more function's

class User{
private $SQL;

    public function __construct(){
        $this->SQL = SQL::getInstance();
    }

    public Function AddUser ($Name, $Password) {
    $sql_str ="INSERT INTO USER SET Name = '$Name', Password='$Password'";
    $this->SQL->do_sql($sql_str);
    }

    public Function DelUser($UserID){
      $sql_str = "DELETE FROM User WHERE UserID = '$UserID'";
      $sql->do_sql($sql_str);

    }

    public Function Login($Login, $Password){
        $sql_str    = "SELECT * FROM User WHERE Login = '$Login' AND Password = '$Password' ";
        LIST($sql_result, $sql_count) = $this->SQL->do_sql($sql_str);
        if ($sql_count == 1){
            $_SESSION["UserID"]  = $this->SQL->result_strip($sql_result, 0, "AdminUserID");
            $_SESSION["Login"]   = $this->SQL->result_strip($sql_result, 0, "Login");
            $sql_str    = "UPDATE User SET LastLogin = NOW()";
            $this->SQL->do_sql($sql_str);

        }
    }

    public Function Logout(){
        $_SESSION = array();
        if (isset($_COOKIE[session_name()])) {
            setcookie(session_name(), '', time()-42000, '/');
        }
        session_destroy();

    }
}
OOP Tester