Define functions in one class

[coreen]

Segment.php

<?php
$mysqli = mysqli_connect("localhost","root","","squarellt");
 $cid = required_param('id', PARAM_INT);
 class segment
 {   
   public function unitseg_set1()
   {          
	 $subject = $mysqli->query('SELECT * FROM order_subject WHERE id='.$cid.'');     
	 while($row=$subject->fetch_array() )
	 {
        echo '<div>'.$row['chem_name'].'</div>';
     }	 
   }
   
   public function unitseg_set2()
   {          
	 $subject = $mysqli->query('SELECT * FROM order_subject WHERE id='.$cid.'');     
	 while($row=$subject->fetch_array() )
	 {
        echo '<div>'.$row['physiotherapy_nn'].'</div>';
     }	 
   }   

   public function unitseg_set3()
   {          
	 $subject = $mysqli->query('SELECT * FROM order_subject WHERE id='.$cid.'');     
	 while($row=$subject->fetch_array() )
	 {
        echo '<div>'.$row['commun_gg'].'</div>';
     }	 
   }   
 }
 ?>

home_segment.php

 <?php
   require_once('segment.php');

   $acc = new segment();
   $account1 = $acc->unitseg_set1();
   $account2 = $acc->unitseg_set2();
   $account3 = $acc->unitseg_set3();
   
   echo $account1;

   echo $account2;

   echo $account3;

I was trying to pullout the content of all the functions defined from the class segment in segment.php. And calling all the three functions from a class segment in home_segment.php Is this a correct way of calling multiple functions from one class.

Or suggest with good OOP's concept. How could I improve the above similar functionality in Object Oriented Programming.

(MOD: added markdown for code blocks)

[maxxd]

You've got some ... well, issues with the posted code.

Part of the point of the concept of OOP is DRY (Don't Repeat Yourself), so ideally you'd not have methods with names like unitseg_set1(), unitseg_set2(), unitseg_set{x}(), but you'd pass in a parameter into a single unitseg_set() function and decide what to return from that.

You're also using the $cid variable and database connection inside class methods without actually passing them into the class.

Your constructor should have the MySQLi connection and $cid variable as parameters - assign those to local properties and you can use them in the class methods. Combine all 3 unitseg_set() methods into one with a return index parameter and use that to decide what value to return from the method.

Honestly, once you get that done you can improve it further by grabbing the database record in your __construct() method, saving that as a local property, and returning the requested value from the cached results. This will avoid unnecessary trips to the database.

[dalecosp]

You're getting the basic idea, but Maxxd's right that you have some issues, and he's given some good advice!

I also note that you're using echo twice and will get duplicate content. You may wish to have the class methods (aka 'functions') return the data instead of echo'ing it.

I also see you're using "SELECT *" in your SQL when you're only returning one field. To avoid that overhead and wasted time, call just the column you're going to return in your query. Also, since you're asking for $row['foo'] (where 'foo' is some string), you're probably going to want fetch_assoc() instead of fetch_array() once you get your result.

Just in case it's tough to wrap your head around it all, here a possible implementation that includes most of his instructions (and mine as well). Sorry about the spacing issues; this board software is fairly new and some of us are having trouble formatting nice clean code with it....

<?php

class segment { 

    private $db;
    private $types;

    public function __construct($mysqli, $cid) {
	
	    if (is_object($mysqli)) {
		
		    $this->db    = $mysqli;
		}
		// allow our method to be called with an integer
		$this->types = array(1=>"chem_name", 2=>"physiotherapy_nn", 3=>"commun_gg")
		
	
	}
    public function unitseg_set($param) { 
	
        if ($int = intval($param)) {
		     
	    $param = $this->types[$int];
	}

        $subject = $this->db->query("SELECT `$param` FROM order_subject WHERE id='" . $cid . "';"); 
        $string  = '';

        while($row=$subject->fetch_assoc()) {

             $string .= "<div>" . $row[$param] . "</div>\n";
         }
         return $string;
    }

}

Your home_segment.php would look like this (unless you really need the data in $account1, $account2, etc. to persist beyond the echo):

<?php

require_once($some_file_with_mysqli_defined_in_it);
require_once($some_file_with_cid_defined_in_it);
require_once('segment.php');

$reports_needed = array(1, 2, 3);

$acc = new segment($mysqli, $cid);

foreach ($reports_needed as $n) {

    echo $acc->unitseg_set($n);
}

Hope this helps. 🙂

[NogDog]

Without a better understanding of the problem space and what a "segment" object is actually representing, my actual approach might vary a lot. Here's just one possible variation based on an assumption (likely wrong?) that you'd only ever be interested in those 3 columns, and providing a way to keep the code pretty D.R.Y. while hopefully enhancing readability for anyone maintaining it:

segment.php:

<?php

class segment
{
    private $mysqli;

    public function __construct(mysqli $mysqli)
    {
        $this->mysqli = $mysqli;
    }

    public function chem_name($id)
    {
        return $this->query('chem_name', $id);
    }

    public function physiotherapy_nn($id)
        return $this->query('physiotherapy_nn', $id);
    }

    public function column_gg($id)
    {
        return $this->query('commun_gg', $id);
    }

    private function query($column, $id)
    {
        $stmt = $this->mysqli->prepare("SELECT $column FROM order_subject WHERE id = ?");
        $stmt->bind_param('i', $id);
        $result = $stmt->execute();
        // may not need this looping if it would only ever return 0 or 1 row?
        $data = array();
        while($row = $result->fetch_array()) {
            $data[] = $row[0];
        }
        return $data;
    }
}

sample usage:

require_once 'segment.php';
// assume $mysqli object created here or at some earlier point or include/require, then...
$segment = new segment($mysqli);
$cid = something(); // No idea where this comes from?
$physiotherapies = $segment->physiotherapy_nn($cid)
if(empty($physiotherapies)) {
    echo "<div class='error'>No data found</p>\n";
}
else {
    foreach($physiotherapies as $value) {
        echo "<div>$value</div>\n";
    }
}

[maxxd]

Combining both dalecosp and NogDog's approaches, I'd go a bit of a different way with it:

class Segment{
/**
 *	@var	MySQLi	Database connection object
 */
	private $_conn;
/**
 *	@var	int		Record UID
 */
	private $_cid;
/**
 *	@var	array	Returned row from the database, indexed by column name
 */
	private $_row;
/**
 *	Create this object.
 *	This will take the database connection and record identifier and query
 *	the database, priming the local row property. This creates a sort of 'cache'
 *	for the data that can be called at any time.
 *	@param	MySQLi			database connection
 *	@param	int			record ID
 *	@return	self
 */
	public function __construct(mysqli $conn, $cid){
		$this->_conn = $conn;
		$this->_cid = intval($cid);
		$this->_row = $this->getRow();
	}
/**
 *	Returns the value for the requested column in the specified row of the database
 *	@param	string			database column name
 *	@return	mixed
 */
	public function getUnit($idx){
		if(!empty($this->_row[$idx])){
			return $this->_row[$idx];
		}
	}
/**
 *	Query the database to prime the data stash
 *	@return	array
 */
	private function getRow(){
		$qry = "
			SELECT	 *
			FROM order_subject
			WHERE id = ?
			LIMIT 0, 1
		";
		$sql = $this->_conn->prepare($qry);
		$sql->bind_param('i', $this->_cid);
		return $sql->execute()->fetch_assoc();
	}
}

Usage:

$conn = mew mysqli([...]);
$reports = [
	'chem_name',
	'physiotherapy_nn',
	'commun_gg'
];

$segement = new Segment($conn, 15);
foreach($reports as $r){
	print("<p>{$segment->getUnit($r)}</p>");
}

This does grab all the columns from the record, which may not be the most efficient use of the resource, but it also negates any additional trips to the database by storing the results. Which method is better is going to depend on how much information you're using or how many different rows you have to grab.