class design improvement - tweet - decoupling super global variables? [closed]

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 9 years ago.

Improve this question

Question:

Should I decouple the SESSION variables, i.e. move them outside of the class...this was a suggestion from another POST. I would like to extend this library to more general cases than just开发者_高级运维 for my purposes..and I believe this is the next step here?

Summary:

This class sends "tweet" data to the client in the form of a custom markup language.

It is used in Ajax call and in previous post people have suggested not to echo the result..but this is my primary means of communicating with the server via Ajax responseText.

Also <tw_p> is used to denote a pass and is read by the client. The markup looks like this.

field 1 | field 2 | field 3 | field 4 || field 1 | field 2 | field 3 | field 4 ||

It is called like this -

new tweet();

The client knows how render this into xhtml once it receives it.

/*tweet*/

class tweet extends post
  {
  function __construct()
    {
    parent::__construct();
    $email=$_SESSION['email'];
    $flname=$_SESSION['name'];
    $message=$this->_protected_arr['f4b'];
    $time=time();
    database::query("INSERT INTO tw VALUES ('$time','$flname','$message','$email')");        
    $query_return = database::query("SELECT * FROM tw ORDER BY time DESC LIMIT 7");
    $b=0;
    $c='<tw_p>';
    while($a=mysqli_fetch_assoc($query_return))
      {  
      if($b==0)
        {
        $c = $c . $a['email'] . "|" .  $a['fname'] . "|" . $a['time'] . "|" . $time . "|" . $a['message'];
        }
      else
        {
        $c = $c . "||" . $a['email'] . "|" .  $a['fname'] . "|" . $a['time'] . "|" . $time . "|" . $a['message'];
        }
      $b++;
      }
    echo $c;
    }
  }

---

What I would do if it was my code:

1. Remove logic from constructor, it is for initialization only. Thus remove all the business logic into separate method doTheWork (choose the meaningful name that explains what happens in the code)
2. Parametrize constructor with:
   • email
   • name
   • database instance (yes, I'd avoid static database::query() method)
3. Remove echo $c; in the end and replace it with return $c;
4. Remove the presentation logic from this class outside (the lines with || stuff)
5. Sanitize the data used in the query (thanks to vzwick)