HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Custom Class to show tweets

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
customclasstweetsshow

Problem

I'm busy on a site, and we've made a custom Twitter class. The tweets are send from our panel, where they're posted and saved in the DB. In this class, we get the tweets from the database and show them on our site.

These are my helping points:

  • Is this code as efficient as possible?



  • Is there a easier way to achieve the same result?



  • Is there a faster way to achieve the same result?



```
connection->prepare('SELECT * FROM tweet ORDER BY id DESC LIMIT 5');
$stmt->execute();
$result = $stmt->get_result();
if ($stmt->num_rows > 0)
{
while ($row = $result->fetch_assoc())
{
$this->setTweetData('naam', $row['naam']);
$this->setTweetData('tweet', $row['tweet']);
$this->setTweetData('dag', $row['dag']);
}
}
$stmt->close;
}

private function setTweetData($key, $value)
{
$this->tweetData[$key] = $value;
}

private function getTweetData($key)
{
return $this->tweetData[$key];
}

private function createTweets()
{
if ($this->ctweet == 5)
{
$style = "border-bottom: 1px solid #666666;";
}
else
{
$style = '';
}

if (strlen($this->getTweetData('tweet')) content .= 'style . '
background-color: #fff;">
top . '
left: -3px;"
src="http://www.habbo.nl/habbo-imaging/avatarimage?hb=img&user=' . $this->getTweetData("naam") . '&direction=3&gesture=sml" />

' . $this->getTweetData("naam") . ': ' . $this->getTweetData("naam") . '

';

if (date("d-m-Y", strtotime($this->getTweetData('dag'))) == date('d-m-Y'))
{
$t

Solution

I'm not quite sure that I understand all parts of your code. It doesn't seem that you are ever printing the actual tweet ($row['tweet']), only the name of the person tweeting? And it also seems to me that you are fetching 5 semi-random tweets (as they are sorted by id, not date), but only using the data of the last one of those (setTweetData overrides the previously set data)?

$ctweet bug

$ctweet doesn't do anything right now. $ctweet++; stands after the return statements and is thus never executed.

Save value in temporary variable

This code:

date("d-m-Y", strtotime($this->getTweetData('dag')))


exists three times (which makes the code hard to read), and if it is an old tweet also gets executed three times (which is slow).

Just save it in a temporary variable.

Escape output

Right now, you are not escaping the data that you print ($this->getTweetData("naam")). You might want to escape it to prevent xss attacks.

Style Information

I would move the style information to an external CSS file. It is faster (because it can be cached separately from the HTML), and also leads to easier to read code.

Code Snippets

date("d-m-Y", strtotime($this->getTweetData('dag')))

Context

StackExchange Code Review Q#63487, answer score: 2

Revisions (0)

No revisions yet.