PHP OOP Help

PdVortex · November 19, 2009

Hello i need some help with a tiny bit of code which i've been trying to solve on my own for well over 7 hours and to be honest i've exhausted all my ideas... and i bet the solution is so simple when someone helps me i will slap myself in the face with a yellow pages book!

So here goes :

This is my output code :

echo ' Here are your latest forum topics : <br />';
$Topics = $user->forumTopics();
echo $Topics;

Which should output the latest topic from a forum that a user has made!

This is the code from the class into which the above code is refereing too :

	function forumTopics() {
	$sql = "SELECT * FROM forum_topic WHERE forum_topic.user_id='".$this->userId."' ORDER BY datetime ASC LIMIT 5";
		$result=$this->db->query($sql);
	while ($row = $result->fetch()){
		return $this->Topics=$row[FORUM_TOPIC];
	}
}

Now i know the sql is correct because its outputting one of the recent topics, but ONLY one it should be outputting 5...

Can anyone help me with this. i have tried for loops but nothing i have done works and its really starting to drive me crazy

Thanks in advance guys/girls.

mrMarcus · November 19, 2009

try that:

<?php
function forumTopics() {
$sql = "SELECT * FROM forum_topic WHERE forum_topic.user_id='".$this->userId."' ORDER BY datetime ASC LIMIT 5";
$result=$this->db->query($sql);

while ($row = $result->fetch())
{ $this->Topics .= $row['FORUM_TOPIC'].'<br />'; }

return $this->Topics;
}
?>

emopoops · November 19, 2009

what does $var->$variable

or $var->function() mean? i dont understand ->

roopurt18 · November 19, 2009

Two things.

1) If you want most recent, you should be orderying by: datetime DESC

2) You're issuing a return statement in your while loop. Return causes the function to immediately return to the caller. Thus your function is issuing the query, fetching the first result, and returning.

PdVortex · November 19, 2009

try that:

<?php
function forumTopics() {
$sql = "SELECT * FROM forum_topic WHERE forum_topic.user_id='".$this->userId."' ORDER BY datetime ASC LIMIT 5";
$result=$this->db->query($sql);

while ($row = $result->fetch())
{ $this->Topics .= $row['FORUM_TOPIC'].'<br />'; }

return $this->Topics;
}
?>

Thanks mrMarcus, your solution worked fine apart from its duplicating the output twice so now im getting "test1 test2 test3 test4" "test1 test2 test3 test4" any ideas on that one?

Two things.

1) If you want most recent, you should be orderying by: datetime DESC

2) You're issuing a return statement in your while loop. Return causes the function to immediately return to the caller. Thus your function is issuing the query, fetching the first result, and returning.

Ah yes thank you for pointing that out roopurt18

I was wondering if return was only capable of output one row.

what does $var->$variable

or $var->function() mean? i dont understand ->

I believe the -> just points to an object inside a class an object is a function.

$var->functionGetTime

Someone correct me if im wrong please don't want to confuse anyone

roopurt18 · November 19, 2009

I was wondering if return was only capable of output one row.

return causes a function to end, optionally returning one value to the calling code.

That one value returned can be any PHP data type, including but not limited to:

+ ints

+ strings

+ floats

+ objects

+ arrays

+ arrays of objects

+ arrays of arrays of arrays of objects

etc.

Basically, if you want to return a list or collection of items, then you must build them up as an array inside the function and then return the entire array.

mrMarcus · November 19, 2009

are you sure you are not echo'ing the output twice? no reason the code i provided would execute twice.

@roopurt18 .. i would assume you know, but just to clarify to the OP, an array is not the only option in returning a list of results, as i have written in my code to return a concatenated string of results.

MadTechie · November 19, 2009

If you ran it twice it would..(kinda)

maybe add

$this->Topics = "";

before the while loop

PdVortex · November 19, 2009

I was wondering if return was only capable of output one row.

return causes a function to end, optionally returning one value to the calling code.

That one value returned can be any PHP data type, including but not limited to:

+ ints

+ strings

+ floats

+ objects

+ arrays

+ arrays of objects

+ arrays of arrays of arrays of objects

etc.

Basically, if you want to return a list or collection of items, then you must build them up as an array inside the function and then return the entire array.

Thanks for the info roopurt18 although mrMarcus solution also works fine.

are you sure you are not echo'ing the output twice? no reason the code i provided would execute twice.

@roopurt18 .. i would assume you know, but just to clarify to the OP, an array is not the only option in returning a list of results, as i have written in my code to return a concatenated string of results.

Definitely not I'm new OOP but not that new

If you ran it twice it would..(kinda)

maybe add
$this->Topics = "";
before the while loop

Thank you MadTechie you add-on to mrMarcus's code worked perfectly I give you the final working code below for anyone else who may need help with a problem like this.

function forumTopics() {
	$sql = "SELECT * FROM forum_topic WHERE forum_topic.user_id='".$this->userId."' ORDER BY datetime DESC LIMIT 5";
		$result=$this->db->query($sql);
		$this->FTopics = "";
	while ($row = $result->fetch()){
		$this->FTopics .= $row[FORUM_TOPIC] .'<br />';
	}
		return $this->FTopics;
	}

Thank you all so much for your help, I try to solve things on my own before running and asking for help, I normally solve the problems on my own but this one got the better of me.

Really appreciate you taking the time to help me out.

--Topic marked solved--

Oh and ***Hits myself with the yellow pages***

PdVortex · November 19, 2009

@roopurt18

If i wanted to put the contents of the sql into an array how would i go about that?

mrMarcus · November 19, 2009

you would change your while loop:

while ($row = $result->fetch()){
     $this->FTopics[] = $row[FORUM_TOPIC];
}
return $this->FTopics;

now, $Topics is an array where $Topics = $user->forumTopics();

and by you adding:

$this->Topics = "";

before the loop would dictate that $this->Topics had to be cleared to prevent duplicate content in the output. so in fact, you are populating $Topics twice

Koobi · November 19, 2009

I would rather do it like this:

function forumTopics() {
	$sql = "SELECT * FROM forum_topic WHERE forum_topic.user_id='".$this->userId."' ORDER BY datetime DESC LIMIT 5";
		$result=$this->db->query($sql);
	$FTopics = array(); //initialize as an array
	while ($row = $result->fetch()){
	    //it makes no sense to use $this here	
                    $FTopics[] = $row[FORUM_TOPIC] .'<br />';
	}
		return $FTopics;
	}

this is my reasoning:

* initializing

PHP is a scripting language and like most scripting languages, it is loose typed, in that, you don't need to tell PHP a variable can only store integer or string. a variable in PHP can store any of the types.

but, it is good practice to initialize variables according to their types. at the end of the day, it's about personal preference and how clean you want your code.

so since $FTopics will be storing data as an array, i chose to initialize it as an array.

this also makes the code easy to read...for some reason if i have to initialize it further up in my code, the fact that i initialized it as an array will give me a hint as to what that variable will later contain.

* not using $this

i wouldn't use $this->FTopics unless i need to use this data elsewhere in the class.

if you need the forum topics to be returned to you and discarded, just use a regular variable.

when you use $this, you are involving the whole class as your scope.

if you have not designed or planned this carefully, you might have another variable called $this->FTopics which can overwrite or append the one within your forumTopics() method.

therefore, use $this sparingly and only if you need it or things can get buggy.

also, if you do end up using $this, you should be initializing it outside the methods like:

MyClass
{
    private $FTopics = array()

    public function forumTopics()
    {
        $sql = "SELECT * FROM forum_topic WHERE forum_topic.user_id='".$this->userId."' ORDER BY datetime DESC LIMIT 5";
$result=$this->db->query($sql);
        while ($row = $result->fetch()) {
            $this->FTopics[] = $row[FORUM_TOPIC] .'<br />';
}
return $this->FTopics;
    }
}

PdVortex · November 19, 2009

Hey thanks for both of your responses, just wondering if i pull 2 bits of information from the function and display them.

i.e. $row[FORUM_TOPIC] and $row[FORUM_ID]

I cant figure it out

$FReplys[] = $row[FORUM_R_ID][FORUM_REPLYS] .'<br />';

That dont work but then to be honest i didnt think it would

$FRId[] = $row[FORUM_R_ID];

$FReplys[] = $row[FORUM_REPLYS];

return $FRId;

return $FReplys;

That doesn't work ah why is the little bit of code irritating me sooo much

mrMarcus · November 19, 2009

you would need to concatenate the results:

$FReplys[] = $row[FORUM_R_ID].'|'.$row[FORUM_REPLYS];

then:

<?php
if (is_array ($user->forumTopics()))
{
$Topics = (strpos ($user->forumTopics(), '|') ? explode ('|', $user->forumTopics()) : $user->forumTopics());

//now, display the results;
foreach ($Topics as $topic)
{ echo $topic.'<br />'; }
}
?>

not the way i'd do it really, but it'll work for you 'til i get a chance to go a little further in depth.

EDIT: you don't want to add HTML to you array values, just makes things complicated. simply populate your array with values, then add the HTML when you output the array. i'm not saying that you absolutely shouldn't put HTML in your array's, 'cause there are times when it is very useful, just in this case it's not necessary.

Koobi · November 19, 2009

^^ i believe what mrMarcus said about embedding HTML is true.

regarding concatenating your data with a string, however, if FORUM_TOPIC happens to contain a pipe '|', you will get unexpected results. so, if you want, you could stick your results into an array and iterate through them like this:

your method modified:

    public function forumTopics()
    {
        $sql = "SELECT * FROM forum_topic WHERE forum_topic.user_id='".$this->userId."' ORDER BY datetime DESC LIMIT 5";
$result=$this->db->query($sql);
        while ($row = $result->fetch()) {
            $FTopics[] = array($row[FORUM_TOPIC], $row[FORUM_ID]);
        }
        return $FTopics;
    }

now, the return value will be an array of arrays.

if you need to visualize this to proceed, do this:

print_r($user->forumTopics());

or

var_dump($user->forumTopics());

feel free to warp that output with HTML <pre> tags if it makes it more pleasant to look at.

now, you can access your data like this:

foreach($user->forumTopics() as $index => $record) {
    echo "Record number: $index<br />";
    echo 'Topic: ' . $record[0];
    echo 'ID: ' . $record[1];
}

roopurt18 · November 19, 2009

EDIT: you don't want to add HTML to you array values, just makes things complicated. simply populate your array with values, then add the HTML when you output the array. i'm not saying that you absolutely shouldn't put HTML in your array's, 'cause there are times when it is very useful, just in this case it's not necessary.

I agree with this sentiment.

If your function is called ForumTopics(), then it is only expected to return the forum topics. The function itself has no idea on how they will be used. Today they are used in an HTML page. Tomorrow they might be used in XML, Json, or a PDF report. Keep the data and the visual representation separate for as long as possible and then your code will be much more reusable.

PdVortex · November 19, 2009

Hey thanks for your responses, to be brutally honest i had no idea where you were going mrMarcus lol

However Koobi your method works perfectly.

function forumTopics() {
	$sql = "SELECT * FROM forum_topic WHERE forum_topic.user_id='".$this->userId."' ORDER BY datetime DESC LIMIT 5";
		$result=$this->db->query($sql);
		$FTopics = array(); //initialize as an array
				while ($row = $result->fetch()){ //it makes no sense to use $this here
				$FTopics[] = array($row[FORUM_TOPIC], $row[TID]);
				}
			return $FTopics;
	}

And the echoing code :

echo ' Here are your 5 latest forum topics : <br />';
foreach($user->forumTopics() as $index => $record) {
    echo '<a href="../forum/view_topic.php?id='. $record[1] .'">' . $record[0] . '</a><br />';
}

I don't like to put output in a class anyway cheers for the advice and help guys you been great!

Koobi · November 19, 2009

no problem.

mrMarcus was just joining your results together and using a pipe '|' to distinguish between the two pieces of data.

he has a point, that method has its uses too but in my humble opinion, i thought the array method might work out better in your situation.

roopurt18 · November 19, 2009

					$FTopics[] = array($row[FORUM_TOPIC], $row[TID]);

    echo '<a href="../forum/view_topic.php?id='. $record[1] .'">' . $record[0] . '</a><br />';

I consider it best practice to reserve numeric indexes for identical items. In your case, $record[1] is a topic id and $record[0] is a topic title; these are not identical items.

You code will become more clear now (and more importantly 6 months from now) if you use an associative array.

In fact, you can let MySQL and PHP do a little of this work for you!

<?php
function forumTopics() {
        $cols = implode( ', ', array( FORUM_TOPIC, TID ) );
        $sql = "
            SELECT {$cols} 
            FROM forum_topic 
            WHERE forum_topic.user_id='{$this->userId}' 
            ORDER BY datetime DESC LIMIT 5
            ";
            $result=$this->db->query($sql);
            $FTopics = array(); //initialize as an array
                    while ($row = $result->fetch_assoc()){ // I'm assuming a fetch_assoc() method exists, and I'd even prefer to use fetch_object()
                    $FTopics[] = $row;
                    }
                return $FTopics;
        }
?>

PdVortex · November 19, 2009

@ roopurt18

Your method can you explain it a bit more....

If the numerical indexes were the same i wouldn't be able to display the correct data?

How would i display TOPIC_ID and TOPIC_TITLE ?

roopurt18 · November 19, 2009

I'm not talking about the indexes being the same. I'm talking about the data in the array representing the same thing.

Your array is:

array(

0 => 'topic',

1 => 'id'

);

When you reference it later in the calling code, you have to use the indexes 0 and 1. The identifier 0 doesn't tell me anything about the data being pointed at.

Neither does the identifier 1.

Your code will be more clear if your arrays were created as:

array(

'topic' => 'My Pets',

'id' => 1

);

Then when you reference the elements of the array you have to use 'topic' and 'id' and the code becomes self-documenting.

Numeric indexes are appropriate when all of the elements of the array are homogeneous. For example:

$employees = array (

0 => 'Betty',

1 => 'Fred',

2 => 'Suzy'

);

Each element of the array is an employee so we don't need the indexes to tell us anything about the data they index to.

Let me know if that's not clear.

Also, out of curiosity, what underlying API are you using to interact with your database?

PdVortex · November 19, 2009

Yes that way would be more clearer how would i implement that? Like your above code? In which case would i still put the cho like this?

foreach($user->forumPosts() as $index => $record) {
    echo '<a href="../forum/view_topic.php?id='. $record[id] .'">' . $record[topic] . '</a><br />';
}

With out sounding like an idiot... what's an api can you give me an example of one.

roopurt18 · November 19, 2009

The API is the set of function you use to interact with your database.

Are you using mysql_connect(), mysql_query(), etc?

Are you using PDO?

The API you're using determines the answer to your other question.

PdVortex · November 19, 2009

    function connectToDb () {
        // Make connection to MySQL server
        if (!$this->dbConn = @mysql_connect($this->host,
                                      $this->dbUser,
                                      $this->dbPass)) {
            trigger_error('Could not connect to server');
            $this->connectError=true;
        // Select database
        } else if ( !@mysql_select_db($this->dbName,$this->dbConn) ) {
            trigger_error('Could not select database');
            $this->connectError=true;
        }
    }

Is the code im using to connect

    function & query($sql) {
        if (!$queryResource=mysql_query($sql,$this->dbConn))
            trigger_error ('Query failed: '.mysql_error($this->dbConn).
                           ' SQL: '.$sql);
        return new MySQLResult($this,$queryResource);
    }
}

is the code i use to query the db

    function fetch () {
        if ( $row=mysql_fetch_array($this->query,MYSQL_ASSOC) ) {
            return $row;
        } else if ( $this->size() > 0 ) {
            mysql_data_seek($this->query,0);
            return false;
        } else {
            return false;
        }
    }

is the code i use to fetch the info.

roopurt18 · November 19, 2009

<?php
function forumTopics() {
    //
    // When asking the database for data, it's more efficient to ask for
    // only the columns you plan to use.  We are only interested in FORUM_TOPIC
    // and TID.  The next line of code will build a string that looks like:
    //      FORUM_TOPIC, TID
    // So that our select will be:  SELECT FORUM_TOPIC, TID FROM ...
    // rather than:  SELECT * FROM ...
    // 
    // With the SELECT * lingo, consider what would happen if you added an extra
    // column to your table that had an average of 2MB of data in it.
    // Every time you called this function and ran your query, you'd be asking
    // the database for an *extra* 2MB of data *per* topic!  If you had only
    // 10 topics, you'd be pushing an extra 20MB of data between your app and
    // the database engine when less than 200KB is probably needed!
    $cols = implode( ', ', array( FORUM_TOPIC, TID ) );
    
    $sql = "
        SELECT {$cols} 
        FROM forum_topic 
        WHERE forum_topic.user_id='{$this->userId}' 
        ORDER BY datetime DESC LIMIT 5
    ";
    $result=$this->db->query($sql);
    $FTopics = array(); // Initialize your return value -- not necessary but good practice!
    
    if( $result ) { // Error checking!  Only proceed if the $result is good
        while ($row = $result->fetch()){ // Your fetch here is calling mysql_fetch_assoc() under the hood, thus $row is already an array
            print_r( $row );// REMOVE THIS LINE LATER
            $FTopics[] = $row;
        }
    }
    return $FTopics;
}?>

I've added changes and comments. Let me know if anything is unclear.

This is based off koob's code above.

Sign In

PHP OOP Help

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Important Information