1
\$\begingroup\$

Here are the 3 files that make up my MVC. The two view files containing the HTML are not included.

Any advice on how to improve this so it is usable by others.

It is meant to be a light-weight alternative to heavier libraries.

Thanks!

<?php // index.php
  /*
  When the user hits reload button on browser
  */
  include 'arche_control.php'; 
  $controller = new controller_reload();
  $controller->reload();
?>


<?php  //model.php
  /* 
     Used to decouple super global variables
     Gateway file for Ajax and File Uploads
  */
  include 'arche_control.php'; 
  if(isset($_FILES['ufile']['tmp_name'])) 
    {
    $controller = new controller_file();
    }
  elseif(isset($_POST['a']))
    {
    $controller = new controller_ajax($_POST['a']);
    }
?>

<?php //  mvc.php

/****************************************************************
Light MVC Framework - copyright 2011 ArcdeV, Inc.
Note the modules are broken up using commented sections not files
as is common
*****************************************************************/

session::start();

/****************************************************************
CREDENTIALS
*****************************************************************/

define('DB_USER', 'archemar_1');
define('DB_PASS', 'A*99');
define('DB_HOST', 'localhost');
define('DB_DATABASE', 'archemar_1');

/****************************************************************
MODEL CLASSES
one_db, database, post, session, validate, signin, signup, 
signout, bookmark, tweet, table_maintain, upload, import
****************************************************************/

/*one_db*/

class one_db 
  { 
  protected static $db; 
  private function __construct() 
    { 
    self::$db=new mysqli(DB_HOST, DB_USER, DB_PASS, DB_DATABASE);
    } 
  public static function get() 
    { 
    if(self::$db==NULL) 
      { 
      new self(); 
      } 
    return self::$db; 
    } 
  }

/*database*/

class database extends one_db      
  {
  function __construct()  
    {
    one_db::get();
    }
  public function query($query) 
    {
    return one_db::$db->query($query);
    }
  private function ref_arr(&$arr)  // will need this later. 
    { 
    $refs = array(); 
    foreach($arr as $key => $value) 
      {  
      $refs[$key] = &$arr[$key];
      } 
    return $refs;
    }   
  }

/*post*/

class post extends database
  {
  public $_protected_arr=array();
  protected function __construct()
    {
    parent::__construct();
    $this->protect();
    }
  protected function protect()
    {
    foreach($_POST as $key=>$value)
      {
      /* magic_quotes issue
      $this->_protected_arr[$key]=mysqli_real_escape_string($value);
      */
      $this->_protected_arr[$key]=$value;
      }
    }
  }

/*session*/

class session
  {
  public static function start()
    {
    session_start();
    }
  public static function activate($a,$b)
    {
    $_SESSION['email']=$a;
    $_SESSION['name']=$b;
    }
  public static function deactivate()
    {
    unset($_SESSION['email']);
    unset($_SESSION['name']);
    }
  public static function active()
    {
    return isset($_SESSION['email']);
    }
  public static function finish()
    {
    $_SESSION=array();
    if(session_id() != "" || isset($_COOKIE[session_name()]))
      {
      setcookie(session_name(),'',time()-2592000,'/');
      }
    session_destroy();
    }
  }

/*validate*/

class validate
  {
  private $input;
  function __construct($input_arg)
    {
    $this->input=$input_arg;
    } 
  function empty_user()    
    {
    if((int)!in_array('',$this->input,TRUE)) return 1;
    return 0; 
    }
  function name()          
    {
    if(preg_match('/^[a-zA-Z-\.\s]{1,60}$/',$this->input['name'])) return 1;
    return 0; 
    }
  function email()
    {
    if(preg_match('/^[a-zA-Z0-9._s-]{1,256}+@[a-zA-Z0-9.-]+\.[a-zA-Z]{1,4}$/',$this->input['email'])) return 1;
    return 0; 
    }
  function pass()
    {
    if(preg_match('/.{6,40}/',$this->input['pass'])) return 1;
    return 0;
    }
  }

/*signin*/

class signin extends post
  {
  function __construct()
    {
    parent::__construct();
    $this->invoke();
    }
  function invoke()
    {
    $obj=new validate($this->_protected_arr);
    if($obj->empty_user())
      {
      if($obj->email())
        {
        if($obj->pass())
          {
          if(self::validate())
            {
            self::activate_session();
            $control=new controller_control();
            $control->send('pass');
            }
          else
            {
            new view_message('validate');
            }
          }
        else
          {
          new view_message('pass');
          }
        }
      else
        {
        new view_message('email');
        }
      }
    else
      {
      new view_message('empty');
      }         
    }
  private function validate()
    {
    $email=$this->_protected_arr['email'];
    $pass=$this->_protected_arr['pass'];    
    $query="SELECT pass FROM cr WHERE email='$email'";
    $row=mysqli_fetch_row(database::query($query));    
    $pass=crypt($pass, $row[0]);
    $query="SELECT email,pass FROM cr WHERE email='$email' AND pass='$pass'";      
    if(mysqli_num_rows(database::query($query))!=0) 
      {
      return 1;
      }
    else
      {
      return 0;
      }
    }
  private function activate_session()
    {
    $email=$this->_protected_arr['email'];
    $res_arr=mysqli_fetch_assoc(database::query("SELECT flname FROM cr WHERE email='$email'"));
    $flname=$res_arr['flname'];
    session::activate($email,$flname);
    }
  }

/*signup*/

class signup extends post
  { 
  function __construct()
    {
    parent::__construct();
    $this->invoke();
    }
  private function invoke()
    {
    $obj=new validate($this->_protected_arr);
    if($obj->empty_user())
      {
      if($obj->name())
        {
        if($obj->email())
          {
          if($obj->pass())
            {
            if(self::taken())
              {
              self::insert_data();
              $control=new controller_control();
              $control->send('pass');
              }
            else
              {
              new view_message('taken');
              }
            }
          else
            {
            new view_message('pass');
            }
          }
        else
          {
          new view_message('email');
          }
        }
      else
        {
        new view_message('name');
        }
      }
    else
      {
      new view_message('empty');
      }         
    }    
  private function taken()
    {
    $email=$this->_protected_arr['email'];
    $query="SELECT * FROM cr WHERE email='$email'";
    if(mysqli_num_rows(database::query($query)))
      {
      return 0;
      }
    else
      {
      return 1;
      }
    }
  private function insert_data()
    {
    $flname=$this->_protected_arr['name'];
    $email=$this->_protected_arr['email'];
    $pass=crypt($this->_protected_arr['pass']);
    database::query("INSERT INTO cr VALUES ('$flname', '$email', '$pass')"); 
    database::query("INSERT INTO bo VALUES ('Facebook','http://www.facebook.com','','$email')"); 
    session::activate($email, $flname);
    }
  }

/*signout*/

class signout
  { 
  function __construct()
    {
    session::finish();
    }
  }

/*bookmark*/

class bookmark extends post 
  {
  function __construct($type)
    {
    parent::__construct();
    if($type=='insert')
      {
      database::query("INSERT INTO bo VALUES ('{$this->_protected_arr["f3a"]}', '{$this->_protected_arr["f3b"]}', '', '$_SESSION[email]')"); 
      $control=new controller_control();
      $control->send('pass');
      }
    else if($type=='delete')
      {
      database::query("DELETE FROM bo WHERE name='{$this->_protected_arr[a1]}' AND email='$_SESSION[email]' LIMIT 1");
      $control=new controller_control();
      $control->send('pass');
      }
    }
  }

/*tweet*/

class tweet extends post
  {
  private $time;
  function __construct()
    {
    $this->time=time();
    parent::__construct();
    $this->invoke();
    }
  private function invoke()
    {
    $this->insert();
    $this->send_aml();    
    }
  private function insert()
    {
    $email=$_SESSION['email'];
    $flname=$_SESSION['name'];
    $message=$this->_protected_arr['f4b'];
    database::query("INSERT INTO tw VALUES ('$this->time','$flname','$message','$email')");        
    }
  private function send_aml()
    {
    $query_return = database::query("SELECT * FROM tw ORDER BY time DESC LIMIT 7");    
    $control=new controller_control(); 
    $aml_string = $control->add('pass');    
    $iter=0;    
    while($a=mysqli_fetch_assoc($query_return))
      {
      if($iter!=0)
        {
        $aml_string = $aml_string . "||";
        }
      $iter++;
      $aml_string = $aml_string . $a['email'] . "|" .  $a['fname'] . "|" . $this->time . "|" . $a['time'] . "|" . $a['message'];      
      }
    echo $aml_string;
    }
  }

/*table_maintain*/

class table_maintain extends database
  {
   public function delete($table_type)
    {
    database::query("DROP TABLE $table_type");
    echo "Table '$table_type' deleted \n";
    }
  private function create($name, $query)
    {
    database::query("CREATE TABLE $name($query)");
    echo "Table '$name' created <br>";
    }  
  public function make($type)
    {
    switch ($type) 
      {
      case "te":
        $this->create('te', 'id INT NOT NULL AUTO_INCREMENT, address VARCHAR(60), date DATE, PRIMARY KEY(id)');
        break;
      case "cr":
        self::create('cr', 'flname VARCHAR(60), email VARCHAR(321), pass VARCHAR(40), INDEX(email(6))');
        break;
      case "bo":
        self::create('bo', 'name VARCHAR(64), url VARCHAR(256), dom VARCHAR(256), email VARCHAR(64)');
        break;
      case "tw":
        self::create('tw', 'time INT, fname VARCHAR(32), message VARCHAR(128), email VARCHAR(64)');
        break;
      case "co":
        self::create('co', 'name VARCHAR(32), number VARCHAR(32), email VARCHAR(32), address VARCHAR(32), web VARCHAR(32), web VARCHAR(32), notes VARCHAR(128), tag VARCHAR(3)');
        break;
      }
    }
  }

/*upload*/

class upload
  {
  private $w, $h, $tw, $th, $max1=50, $max2=20;
  private $path1="images/generic_large.jpg", $path2="images/generic_small.jpg";
  private $src=NULL;
  function __construct()
    {
    $this->invoke();
    }
  private function invoke()
    {
    $email=$_SESSION['email'];
    $path3="pictures/$email.jpg";
    $path4="pictures/$email-1.jpg";
    if(move_uploaded_file($_FILES['ufile']['tmp_name'], $path3))
      {
      if($this->get_image($path3))
        {
        list($this->w,$this->h)=getimagesize($path3);
        $this->tw=$this->w;$this->th=$this->h;
        $this->resize_move($this->max1,$path3);        
        $this->resize_move($this->max2,$path4);        
        imagedestroy($this->src);
        }
      }
    else
      {
      copy($this->path1, $path3);
      copy($this->path2, $path4);
      }
    }
  private function get_image($path)
    {
    $a=TRUE;
    switch($_FILES['ufile']['type'])
      {
      case "image/gif":   
        $this->src = imagecreatefromgif($path); 
        break;
      case "image/jpeg":    
      case "image/pjpeg":   
        $this->src = imagecreatefromjpeg($path); 
        break;
      case "image/png":   
        $this->src = imagecreatefrompng($path); 
        break;
      default:          
        $a = FALSE; 
        break;
      }
    return $a;

  $type_creators = array( 
    'image/gif' => 'imagecreatefromgif', 
    'image/pjpeg' => 'imagecreatefromjpeg', 
    'image/jpeg' => 'imagecreatefromjpeg', 
    'image/png' => 'imagecreatefrompng');

  $img_type = $_FILES['ufile']['type']; 

  if(array_key_exists($img_type, $type_creators)) 
    { 
    $this->src = $type_creators[$img_type]($path); 
    return true; 
    } 
  return false; 
    }
  private function resize_move($max, $path)
    {
    if($this->w > $this->h && $max < $this->w)  
      {
      $this->th = $max / $this->w * $this->h;   
      $this->tw = $max;
      }  
    elseif($this->h > $this->w && $ax < $this->h)
      {
      $this->tw = $max / $this->h * $this->w;
      $this->th = $max;
      } 
    elseif($max < $this->w)
      {
      $this->tw=$this->th=$max;
      }
    $a = imagecreatetruecolor($this->tw, $this->th);
    imagecopyresampled($a, $this->src, 0, 0, 0, 0, $this->tw, $this->th, $this->w, $this->h);           
    imagejpeg($a, $path);
    imagedestroy($a);   
    }
  }

/*import*/

class import extends database
  {
  protected function __construct($file_name)
    {
    parent::__construct();
    $this->ie($file_name);
    }
  function ie($file_name)
    {
    $lines = file('ie.htm');
    foreach ($lines as $line) 
      { 
      if(preg_match("/(https?:\/\/.+?)\"(.+)\" >(.+)\./", "$line", $matches))
        {
        //('bo', $matches[3], $matches[1] );
        }
      }
    }
  }

/****************************************************************
VIEW MODULE
view, view_database, view_message
view_arche_1 (external), view_arche_2 (external) - these are 
  primarily html files 
****************************************************************/

/*view*/

class view extends database
  {
  }

/*view_db*/

class view_db extends view
  {
  function __construct($type)
    {
    parent::__construct();    
    $this->invoke($type);
    }
  private function invoke($type)
    {
    switch ($type) 
      {
      case "bookmarks":
        $this->html_bookmarks();
        break;
      case "tweets":
        $this->html_tweets();
        break;
      default:
        throw new Exception('Invalid View Type');
        break;
      }
    }
  private function html_bookmarks()
    {
    $email = $_SESSION['email'];
    $query_return = database::query("SELECT * FROM bo  WHERE email='$email' ORDER BY name ASC");
    $html_string='';
    while ($ass_array = mysqli_fetch_assoc($query_return))
      {
      $fav=$this->favicon($ass_array['url']);
      $html_string = $html_string . "<img name='bo_im' class='c' src='$fav' onerror='i_bm_err(this)'><a target='_blank' name='a1' class='b' href = $ass_array[url]>$ass_array[name]</a>";
      }      
    echo $html_string;
    } 
  private function html_tweets() 
    {
    $query_return = database::query("SELECT * FROM tw ORDER BY time DESC LIMIT 7");
    $time = time();
    $html_string='';
    while ($a = mysqli_fetch_assoc($query_return))
      {
      $html_string = $html_string . "<div class='Bb2b'><img class='a' src='pictures/$a[email].jpg' alt=''/><a class='a' href='javascript:void(0)'>$a[fname] posted <script type='text/javascript'>document.write(v0($time,$a[time]))</script></a><br/><p class='c'>$a[message]</p></div>";
      }
    echo $html_string;
    }
  private function favicon($url)     
    {    
    $pieces = parse_url($url);    
    $domain = isset($pieces['host']) ? $pieces['host'] : '';    
    if(preg_match('/(?P<domain>[a-z0-9][a-z0-9\-]{1,63}\.[a-z\.]{2,6})$/i', $domain, $regs))       
      {     
      return $pieces['scheme'] . '://www.' . $regs['domain'] . '/favicon.ico';    
      }    
    return false;
    }
  }

class view_message extends view
  {
  function __construct($type)
    {    
    $this->invoke($type);
    }
  private function invoke($type)
    {
    $this->message($type);
    }   
  private function message($type)
    {
    $type_message = array( 
    'empty' => '<si_f>Please complete all fields.', 
    'name'=> '<su_f>Only letters or dashes for the name field.',
    'email' => '<si_f>Please enter a valid email.',  
    'taken' => '<si_f>Sorry that email is taken.', 
    'pass' => '<si_f>Please enter a valid password, 6-40 characters.',
    'validate' => '<si_f>Please contact <a class="d" href="mailto:[email protected]">support</a> to reset your password.');    
    echo $type_message[$type];
    }
  }

/****************************************************************
CONTROLLER MODULE - controller_file, controller_ajax, 
controller_reload, controller_database, controller_control
****************************************************************/

/*controller_reload*/

class controller_reload
  {
  public $model;
  public function __construct()
    {
    }
  public function reload()
    {
    if(session::active())
      {
      include 'arche_view_2.php';
      }
    else
      {
      include 'arche_view_1.php';
      }
    }
  public function reload_header()
    {
    $uri = 'http://' . $_SERVER['HTTP_HOST'] ;
    header('Location: '.$uri);
    }
  }

/*controller_ajax*/

class controller_ajax
  {
  public $model;
  function __construct($type)
    {
    $this->invoke($type);
    }
  function invoke($type)
    {
    if(isset($type))
      { 
      switch($type) 
        {
          case '0': 
            $this->model = new signin(); 
            break; 
          case '1':
            $this->model = new signup(); 
            break; 
          case '2':
            $this->model = new signout();         
            $controller=new controller_reload();
            $controller->reload_header();
            break; 
          case '3': 
            $this->model = new bookmark('insert');
            break; 
          case '3a': 
            $this->model = new bookmark('delete'); 
            break; 
          case '4': 
            $this->model = new tweet(); 
            break;
          default:
            throw new Exception('Invalid Model Type');          
            break;
        }
      }
    }
  }  

/*controller_database*/

class controller_database
  {
  private $model;
  function __construct($type)
    {
    $this->invoke($type);   
    }
  function invoke($type)
    {
    switch($type) 
      {
      case 'bookmarks': 
        $this->model = new view_db($type); 
        break; 
      case 'tweets':
        $this->model = new view_db($type); 
        break; 
      default:
        throw new Exception('Invalid View Type');           
        break;
      }
    }
  }

/*controller_file*/

class controller_file
  {
  public $model;
  function __construct()
    {
    $this->invoke();
    }
  function invoke()
    {
    $this->model=new upload();
    $controller=new controller_reload();
    $controller->reload_header();
    }
  }

/*controller_control*/

class controller_control
  {
  private  $type_array = array( 
    'pass' => '<xx_p>', 
    'fail' => '<xx_f>');
  function add($type)
    {
    return $this->type_array[$type];
    }
  function send($type)
    {
    echo $this->type_array[$type];
    }
  }

?>
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$
  • This code seems to require changes based on whether magic quotes are enabled. Moreover, it's currently assuming that magic quotes are enabled. One should be encouraging the elimination of magic quotes from this earth, not depending on them.

  • This rather hard-to-decipher code contains no comments. I'm usually against lots of comments, but i'm even more against code whose intent is not self-evident. Pick one or the other. (Allow me to cast my vote here in favor of making the code readable enough that you don't need comments.)

  • Who told you two-char table names were acceptable? Names should be descriptive. You seem to be aware that there's no two-char limit on name lengths; use that to make life easier for the poor schmuck who ends up working on this code later (which could be you in 6 months). I should not need documentation to look through your stuff and see what it does. Particularly considering you haven't provided any. :P

  • A post is a database? No. A view is a database? Even more strongly, no. (You've claimed this is MVC; you should know that in MVC, views should never even have direct access to the db. That's the model's job.) Use inheritance to say something "is a" something-else, not just because it's handy to say parent::__construct() to set up the DB.

  • BTW, that's another thing. The constructor's job is to set up an object. No more, no less. new something() should return me a new something -- that i will use. No more, no less. It should not have side effects outside of setting up the instance in question. Setting some magic global 3 layers up in the inheritance tree? Hell no. You should never have a naked new something();; if you want an object, then you should be using it (storing it, calling functions on it, etc). If you don't, then quit abusing constructors. Hell, if everything is going to be init'ing the database anyway, then just say database::get() at the beginning of the code and spare yourself all the unnecessary inheritance.

  • What's up with the odd tags in the strings in view_message::message, controller_control::$type_array, ? You know that's not valid HTML. If that's meant to be interpreted by JS or something, use a standard format like JSON.

  • Stylewise, your indents and naming scheme suck. It's almost universal convention that class names start with an uppercase letter. And i've seen precisely two people in my life who align braces like that -- neither of whom i'd ever let touch any code i'll ever be working on. It always looks to me like someone misplaced their braces. Google "php coding style" and read up on conventions -- almost all recommend 4 space indents, and K&R, OTBS, or Allman style braces. If you intend to release this code to the world, you'd better stick to common conventions. (The only plus is, at least you're consistent about it.)

\$\endgroup\$
14
  • \$\begingroup\$ Great Advice. Although not set in stone, I'd recommend Zend's coding standard. Who better than the makers of the language to follow for it. \$\endgroup\$ Nov 8, 2011 at 17:55
  • \$\begingroup\$ magic quotes is set by my serverice provider...I can not change this..I can however use bound queries as opposed to escaping..this is on my todo list \$\endgroup\$
    – user7459
    Nov 12, 2011 at 18:53
  • \$\begingroup\$ I will make the code more readable....likewise with the table names \$\endgroup\$
    – user7459
    Nov 12, 2011 at 18:53
  • \$\begingroup\$ I'm going to change the comments from MVC to MV...as I do not follow the MVC pattern..it would add small in-efficiencies...and I can not see a benefit in adhering to it...I will provide a UML like diagram so it is easy to understand on the spot. \$\endgroup\$
    – user7459
    Nov 12, 2011 at 18:58
  • \$\begingroup\$ I belive contstructors can be more abstact...I learned this from quantlib.org...though I don't have a strong opinion on this I will consider more to the return type necessity \$\endgroup\$
    – user7459
    Nov 12, 2011 at 18:59
2
\$\begingroup\$

Some random thoughts:

1, You should put your database connection informations (and other settings) to a config.php.

2, Using inheritance to share code generally isn't a good idea: What are the disadvantages of using inheritance as a way of reusing code?

3, Instead of the magic number 2592000 use a named constant.

4, I'd write the invoke() method in this way:

function invoke() {
    $obj=new validate($this->_protected_arr);
    if(!$obj->empty_user()) {
        new view_message('empty');
        return;
    }

    if(!$obj->email()) {
        new view_message('email');
        return;
    }

    if(!$obj->pass()) {
        new view_message('pass');
        return;
    }
    ...
}

It's much easier to read.

\$\endgroup\$
2
  • 2
    \$\begingroup\$ 2. Depends on the circumstances. It's acceptable to use inheritance to share functionality if a tweet is, indeed, a post (for example); one shouldn't need to write similar code twice. But a post is not a database, and trying to say it is one will lead to trouble. Like, say, a tweet accidentally being passed in as a database. \$\endgroup\$
    – cHao
    Nov 7, 2011 at 16:59
  • \$\begingroup\$ I agree. It's worth reading Item 16: Favor composition over inheritance from Effective Java Second Edition. \$\endgroup\$
    – palacsint
    Nov 7, 2011 at 20:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.