A method to insert a database entry for a newly registered account I've started to learn an OOP and I've built a class called accountactions, and I would like to know if I did a good job writing it.

The class is in the file: accountactions.class.php.

&lt;?php

class accountactions(){

    public function register($login, $password, $email){

        //Zapisujemy dane wysłane formularzem
        $this-&gt;username = mysql_real_escape_string($login);
        $this-&gt;password = mysql_real_escape_string($password);
        $this-&gt;email = mysql_real_escape_string($email);

        //Hash password
        $this-&gt;password = md5(sha1($this-&gt;password));

        $db-&gt;simplequery("INSERT INTO radio_users(id, username, password, email) VALUES('', '$this-&gt;username', '$this-&gt;password', '$this-&gt;email')");

    }


}

?&gt;


The register.php file:

&lt;?php

    require_once("accountactions.class.php");

    $account = new accountactions();

    $account-&gt;register('samplelogin', 'samplepassword', 'sample@email');

?&gt;


And I am having some issues with this fragment:

$db-&gt;simplequery("INSERT INTO radio_users(id, username, password, email) VALUES('', '$this-&gt;username', '$this-&gt;password', '$this-&gt;email')");


How do I join my db class to my account class?

I'd like to keep a model where I can do something like:

$account-&gt;register('$_POST['login']', '$_POST['password']', '$_POST['email']');


Unless there is a better way to do this.

Im newbie in OOP so any tips and guidelines are appreciated.
Organization, while not exactly code related, is still very important. Especially when it comes to OOP and especially when you're just getting started. If your code is organized correctly, there is no need for namespaces such as ".class". I spent a solid week perusing different frameworks before finally deciding on one I liked. Then I spent another month fine tuning and tweaking it. Originally, my organizational scheme came from the Zend framework.

application
    configs
    controllers
    models
    views
data
    logs
    cache
public
    images
    css
    js


There's a bit more to it, but I got the most important bits. This scheme specifically caters to an MVC framework, so it may not apply to your situation. You don't have to use this scheme, you don't even have to use a popular one. Hell, you can create your own to fit your own needs, and sometimes that is required. However, a good organizational scheme is hard to find and the popular ones are usually popular for a reason. And one thing a good organizational scheme should do is allow you to avoid namespacing your files. In my scheme my class files would go in one of two folders. The "controllers" or the "models". Knowing these were the only locations for class files means I don't have to namespace them.

Sorry for the mini-rant, let's move along now. I don't really see anything wrong with the OOP, but then again, there's not really much going on here. I do have a few suggestions that might help though.

First, your class is incomplete. You have three class properties and have not declared them. You should always declare your properties, otherwise you risk allowing public access to private properties, such as the password property.

class accountactions() {
    private
        $username,
        $password,
        $emal
    ;


You should avoid internal comments as they just add clutter to your code. If your code is self-documenting then most comments become unnecessary anyways. Everything else can be relegated to doccomments. So, for instance: "Hash password" is pretty obvious from the context and is unnecessary.

Its a minor violation, but still, there is a violation of the "Don't Repeat Yourself" (DRY) Principle here. As the name implies, your code should not repeat itself. You use the same function on each of your method's parameters in order to sanitize them. The easiest way to do this would be to change the way the method accepts its parameters so that you instead accepted an array. Then you could loop or walk over the array to sanitize its contents. For instance:

public function register( Array $credentials ) {
    array_walk( $credentials, 'mysql_real_escape_string' );

    $this-&gt;username = $credentials[ 'username' ];
    //etc...


However, there is a problem with this. For one, these parameters should not be sanitized. Validated: yes; Sanitized: no. You validate for registry; You sanitize for login. If you sanitize them now then you could be changing what that user provided without informing them, which could cause all kinds of confusion. The better thing to do here would be to verify that it has no illegal characters and have the user change it if necessary. Once you validate it, it has essentially been sanitized and you can use it like normal.

Another thing you might consider is using custom sanitizing/validating filters. mysql_real_escape_string() was a very nifty tool when you had nothing better to use, but now there are options such as filter_input() and filter_var(). The former should only be used when accessing raw user input from a form directly, the latter when accepting unknown variables. In this instance, even though you will probably get these values from a form, you will use the latter because your method only sees it as a variable and doesn't know for sure where it comes from.

if( filter_var( $username, FILTER_SANITIZE_STRING ) === $username ) {
    $this-&gt;username = $username;
}
if( filter_var( $email, FILTER_VALIDATE_EMAIL ) ) {
    $this-&gt;email = $email;
}


You might want to consider adding whitespace for legibility on lines that are too long. The rule of thumb here is 80 characters, including indentation. Both PHP and SQL are very lax when it comes to whitespace, so don't be afraid to use it.

$db-&gt;simplequery( "
    INSERT INTO radio_users( 
        id,
        username,
        password,
        email
    ) VALUES (
        '',
        '$this-&gt;username',
        '$this-&gt;password',
        '$this-&gt;email'
    )
" );


Now, as to how you can incorporate your $db class into your accountactions class. The easiest, and best, way is to inject it. This can either be done during construction, or via some sort of setter. I'm going to assume that your database model is still MySQL because I can't find anything to contradict it, so I'll use that to type hint it. By the way, you should really upgrade your MySQL to MySQLi or PDO and take advantage of their prepared statements if you haven't already done so.

public function __construct( MySQL $db ) {
    $this-&gt;db = $db;
}

/** or as a setter */
public function setDb( MySQL $db ) {
    $this-&gt;db = $db;
}


Hope this helps!