Jump to content

Could I get some feedback on this?


mdmartiny

Recommended Posts

I am in the process of writing a class to get the ip address and the referring site to store in a database. I am not all that familiar with writing classes and want to make sure that I am on the right path. If I am doing something wrong in it please explain it to me so I can learn

class browser_ip_ref{
		
		private $_db;
		private $_ip;
		private $_url;

	    public function __construct($db,$ip,$url){
	        $this->_db = $db;
	        $this->_ip = ip2long($_SERVER["REMOTE_ADDR"]); 
	        $this->_url = $_SERVER['HTTP_REFERER'];
	    }

	    function getIp(){
	        if($this->_ip){
	            if (!empty($_SERVER['HTTP_CLIENT_IP'])) {
	                $this->_ip = $_SERVER['HTTP_CLIENT_IP'];
	            } elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
	                $this->_ip = $_SERVER['HTTP_X_FORWARDED_FOR'];
	            }
	            return $this->_ip;
	        }
	    }

	    function insertInfo($this->_ip, $pageID, $browser, $version){
	    	$country_code = $this->_db->query("SELECT `ci` from `ip` WHERE ".$this->_ip." BETWEEN `start` AND `end`");
	        $country_code = $country_code->fetch_array(MYSQLI_ASSOC);
	        $countryID = $country_code['ci'];

	        $ipCount = $this->_db->("SELECT COUNT(*) AS count from `hit_counter_info` WHERE `longIP` = ".$this->_ip." AND `pageID` = $pageID");
	        $total = $ipCount->fetch_array(MYSQLI_ASSOC);
	        $num_rows =  $total['count'];

	        if ($num_rows < 1) {
	            //if not in the database add it.
	            $this->_db->("INSERT INTO `hit_counter_info` (`longIP`,`pageID`,`cc`,`browser`,`version`) VALUES ('".$this->_ip."', '".$pageID."', '".$countryID."', '".$browser."', '".$version."')");
	        }
	    }

	    function insertRef(){
	        $url_parts = parse_url($this->_url);
	        $host_parts = explode( '.', $url_parts['path']);
	        $tld = end($host_parts);
	        $good_tld = array('com','org','edu','uk','net','ca','mil','de','jp','fr','au','us','ru','ch','it','nl','se','no','es');

	        if (empty($url)) {
	            $this->_db->("UPDATE `hit_counter_referrers` SET `ref_count` = ref_count + 1 WHERE `ref_url` = 'bookmarked'");
	        } else if (in_array($tld, $good_tld)) {
	            $rowCount = $this->_db->("SELECT COUNT(*) AS count from `hit_counter_referrers` WHERE `ref_url` = '$this->_url'");
	            $total = $rowCount->fetch_array(MYSQLI_ASSOC);
	            $num_rows =  $total['count'];

	            if ($num_rows > 0) {
	                $this->_db->("UPDATE `hit_counter_referrers` SET `ref_count` = ref_count + 1 WHERE `ref_url` = '$this->_url'");
	            } else {
	                $this->_db->("INSERT INTO `hit_counter_referrers` (`ref_url`,`ref_count`) VALUES ('$this->_url', 1)");
	            }
	        }
	    }
	} 
Link to comment
Share on other sites

public function __construct($db,$ip,$url){
	$this->_db = $db;
	$this->_ip = ip2long($_SERVER["REMOTE_ADDR"]); 
	$this->_url = $_SERVER['HTTP_REFERER'];
}
You accept an $ip and $url but don't use them. If you want them as defaults that's one thing,

public function __construct($db,$ip=null,$url=null){
	$this->_db = $db;
	$this->_ip = ip2long($ip ?: $_SERVER["REMOTE_ADDR"]);
	$this->_url = ($url ?: $_SERVER['HTTP_REFERER']);
}
but if not then don't have them at all.

 

function getIp(){
	if($this->_ip){
		if (!empty($_SERVER['HTTP_CLIENT_IP'])) {
			$this->_ip = $_SERVER['HTTP_CLIENT_IP'];
		} elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
			$this->_ip = $_SERVER['HTTP_X_FORWARDED_FOR'];
		}
		return $this->_ip;
	}
}
Think you got your logic a bit messed up. Plus you're setting $_ip in the constructor so most of this is irrelevant, or even better should be moved into the constructor instead.

 

function insertInfo($this->_ip, $pageID, $browser, $version){
"$this->_ip" doesn't belong in there. (Didn't look at the rest of the code in there.)

 

 

A point about objects: initialize everything you can in the constructor, and pass in as arguments whatever information is needed to do that. Here that would be the database connection at a minimum, and IP address and URL are optional (as I mentioned earlier).

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.