I'm trying to use a session var and call the logged in users details through object on the page. The user has already logged in through my login object. Here are my objects:

class User {
   public $userdata = array();


//instantiate The User Class
   public function User(){  }

public function set($var, $value) {
    $this->userdata[$var] = $value; 
}

public function get($var) {
    if(isset($this->userdata[$var]))
    {
        return $this->userdata[$var];
    }
    return NULL;
}

  function __destruct() {
    if($this->userdata){
       $this->userdata;
    }
  } 
}

class UserService {
private $db;
private $fields;
public $user;
private $session;


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

    }



//  //get current user by ID
    public function getCurrentUser($session) {

            $this->session = $session;

        $query = sprintf("SELECT * FROM User WHERE idUser=%s",
        $this->db->GetSQLValueString($this->session, "int"));
        $result = $this->db->query($query);

         if($result && $this->db->num_rows($result) > 0){

            //create new user
                $user = new User();
                $row = $this->db->fetch_assoc($result);

            //set as object
            foreach($row as $key => $value) {
                $user->set($key, $value);

                break;
            }
            return $user;
            //return $this->user;

        }
        return NULL;    


    }
 }

On my page I've check my session var has a value which it does, so I call the object like so.

 $um = new UserService();
 $user = $um->getCurrentUser($_SESSION['MM_Username']);
 echo $user->get('UserSurname');

however, I see no user surname on the page. I have checked with a none object query and I see a surname but as soon as its object is doesn't work.

I think the problem is here:

foreach($row as $key => $value) {
                $user->set($key, $value);

                break; // you should probably remove it
            }

You should use unnecessary break and probably after setting for example id you stop setting another object properties (UserSurname, Name and so on).

In addition it's quite confusing that inside $_SESSION['MM_Username'] you store idUser and not UserName

Code review

Marcin Nabialek allready answered your question. That break in the foreach is. well. what is it doing there?

But, there are much more things broken in your code. So here is a code review:

Constrcutors

You obviously know what a constructor is. You use it in both classes. But, differently. why? You User class has a public function User() but your UserService has a public function __construct(). Pick one, and stick to it. And if you can choose, pick the correct one: __construct()

From the phpdoc:

As of PHP 5.3.3, methods with the same name as the last element of a namespaced class name will no longer be treated as constructor. This change doesn't affect non-namespaced classes.

So namespacing your User class will break your constructor. This may not be a problem now, but it smells. Simply use __construct(). It is the prefered and correct way to do it. We live today, and not in the past of php4- days :)

Code styling

Oh god, a lot of kittens died today!

Sometimes you have a bracket on a new line:

if (isset($this->userdata[$var]))
{
    return $this->userdata[$var];
}

and sometimes you don't

if($this->userdata){
    $this->userdata;
}

Again, pick something and stick to it. and if you want to save some kittens. stick to the standards: PSR-1 & PSR-2

Public atributes, jeuk

Your User class has a public var $attributes. So it is accessible from the outside world. But you also give us a get and set method. why?

A good rule is: public $var smells, protected $var should be used with caution and private $var is the good stuff.

What are your classes? and why don't you use __constructors?

If I look at the User class, I always look at the __constructor. The User class needs no variables. So I should be able to do something like this:

$me = new User();
$me->getName(); //who am I ?

This ofcourse doesn't work. A User without a name doesn't make sense. It always has one. So ask for it!

class User
{
    public function __construct($name)
    {
        $this->name = $name;
    }
}
$me = new User('jeroen');
$m->getName(); //I am jeroen :)

If you need something ask for it ;) So:

public function __construct(Database $db)

Is the way to roll!

Don't make me read the database

Now, your get/set methods are tigthly coupled with your database. If you change the name of a column in the database. You can refractor your entire code to get('new_column_name'); . Sounds like fun!

Also, what does the method say me? Nothing, does it write easy? no

getName says what it does, it gets me the name.

get tels me i'm getting something. but what?

other questions rise: get('name') =?= get('Name')

It's ok for the User object to know what it has.

Summary

Ok, I outlined some things wrong in your code. Some concepts you should look into:

• SOLID
• PSR standards
• Factory Pattern (this will help you with your UserService
• Inversion Of Control

So, for the sake of the article, here is your code revamped. Note that I wrote it into this commentbox directly, so I could have missed some things and made some errors.

Changelist:

• I cleaned up styling
• I added some comments
• removed _destruct() (it wasn't doing anything)
• Used PDO instead of Databse class (no idea what you are using, but looks like a PDO wrapper)
• changed some table names and the select query (never use * in a selct query. Onyl ask for that what you need)
• used prepared statements
• more flexibility
• exceptions instead of returning null;
• Your $DBH could be put into a singleton/factory to ease your $dbh creation: Database::getInstance(); or DatabaseFactory::getInstance()->createPDO(); or so. Long time since I wrote something like this

Usage:

$DBH = new PDO("mysql:host=$host;dbname=$dbname", $user, $pass);

$userRepository = new UserRepository($DBH);

$id = $_SESSION['MM_Username'];

try
{
    $me = $userRepository->find($id);
}
catch( UserNotFoundException $e )
{
    //user not found
}

print $me->getSurName();

User class:

class User
{

    /**
     * If the User persists in the DataBase
     * $id holds it's db id
     * @var int
     */
    private $id;

    /**
     * @var String
     */
    private $surName;

    /**
     * @var String
     */
    private $firstName;

    /**
     * @param String $firstName
     * @param String $surName  
     * @param int    $dbId        
     */
    public function __construct($firstName, $surName, $dbId=null)
    {
        $this->id = $dbId;
        $this->firstName = $surName;
        $this->surName = $surName;
    }

    /**
     * Does the user ecist in the DB?
     * @return boolean
     */
    public function hasId()
    {
        return $this->id !== null;
    }

    /**
     * @return int
     * @return null   user doesn't persist in DB
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * Return the users full name
     *  The fullname consists of his FirstName and SurNAme
     * @return String
     */
    public function getName()
    {
        return $this->firstName . ' ' . $this->surName;
    }

    /**
     * @return String
     */
    public function getSurName()
    {
        return $this->surName;
    }

    /**
     * @return String
     */
    public function getFirstName()
    {
        return $this->firstName;
    }

    /**
     * Setters: we return $this to allow chaning
     */
    public function setId($id) 
    {
        $this->id = $id;
        return $this;
    }

    // ... other methods here. You can add extra swet stuff.
    // for instance check or it is a valid firstName, or email or ...

    //I removed your __destrouct, because wtf? it isn't doing anything at all
}

and your UserRepository:

/**
 * The UserRepository queries the database and get's you your users
 */
class UserRepository
{

    private $db;

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

    public function find($id)
    {
        $statement = $this->db->prepare('SELECT id,first_name,sur_name FROM users WHERE id = :id');
        $statement->execute(array(
            'id' => $id
        ));

        if ( null === ($user = $statement->fetch(PDO::FETCH_ASSOC)) )
        {
            throw new UserNotFoundException();
        }

        return new User(
            $user['first_name'],
            $user['sur_name'],
            $user['id']
        );
    }

}

and the exception:

class UserNotFoundException extends Exception();