Improving Old Code for new Project

[Xtremer360]

I'm trying to figure out how I can reduce as much functionality as possible and still keep it doing what it needs to do. What I mean by functionality is the number of queries being performed or any of the other code for that matter. I do show at the bottom of my post what I have so far, however that doesn't cover all the queries.

 

 

 

 

<?php
            if ($access_level_id == 2 || $access_level_id == 3) {
       $query = "SELECT
         characters.id
        FROM
         characters"; 
       $result = mysqli_query ($dbc,$query); 
       $total_num_characters = mysqli_num_rows($result);
     
       $query = "SELECT
         user_characters.id
        FROM
         user_characters
        INNER JOIN user_accounts
            ON user_accounts.id = user_characters.user_id"; 
      } else { 
       $query = "SELECT
         user_characters.id
        FROM
         user_characters
        INNER JOIN user_accounts
            ON user_accounts.id = user_characters.user_id
        WHERE
         user_accounts.id = '".$user_id."'"; 
      }
      $result = mysqli_query($dbc,$query);
            echo 

      $num_available_characters = mysqli_num_rows($result);
            if (($num_available_characters > "1") || (($access_level_id == 2 || $access_level_id == 3) && (isset($total_num_characters)) && ($total_num_characters > "0"))) {
            ?>
       <form method="POST" id="change_default_character">
                <select class="dropdown" name="new_default_character_id" id="new_default_character_id" title="Select Character">                
            <?php
    
       if ($default_character_id > "0") {
        print "<option value=".$default_character_id.">".$default_character_name;
       } else {
        print "<option value=0>- Select -";
       }
                if ($access_level_id == 2 || $access_level_id == 3) {
        $query = "SELECT
          characters.id,
          characters.character_name
         FROM
          characters
         WHERE
          characters.id <> '".$default_character_id."' AND
          characters.status_id = '1'
         ORDER BY
          characters.character_name"; 
       } else {
        $query = "SELECT
          characters.id,
          characters.character_name
         FROM
          characters
         INNER JOIN
             user_characters
             ON characters.id = user_characters.character_id      
         INNER JOIN
          user_accounts
             ON user_accounts.id = user_characters.user_id
         WHERE
          user_accounts.id = '".$user_id."' AND
          user_characters.character_id <> '".$default_character_id."' AND
          characters.status_id = '1'
         ORDER BY
          characters.character_name"; 
       }
       $result = mysqli_query ($dbc,$query);
       $num_rows = mysqli_num_rows ($result);
       if ($num_rows > 0) {
        if ($access_level_id == 2 || $access_level_id == 3) {
         print "<optgroup label=\"** Active Characters **\">";
        }
                    while ( $row = mysqli_fetch_array ( $result, MYSQL_ASSOC ) ) {
         print "<option value=\"".$row['id']."\">".$row['character_name']."</option>\r";
        }
       }
                if ($access_level_id == 2 || $access_level_id == 3) {
        $query = "SELECT
          characters.id,
          characters.character_name
         FROM
          characters
         WHERE
          characters.id <> '".$default_character_id."' AND
          characters.status_id = '2'
         ORDER BY
          characters.character_name"; 
       } else {
        $query = "SELECT
          characters.id,
          characters.character_name
         FROM
          characters
         LEFT JOIN
          user_characters
             ON characters.id = user_characters.character_id      
         LEFT JOIN
          user_accounts
             ON user_accounts.id = user_characters.user_id
                        WHERE
          user_accounts.id = '".$user_id."' AND
          user_characters.character_id <> '".$default_character_id."' AND
          characters.status_id = '2'
         ORDER BY
          characters.character_name"; 
       }
       $result = mysqli_query ($dbc,$query); 
       $num_nows = mysqli_num_rows($result);
       if ($num_rows > "0") {
        print "<optgroup label=\"** Inactive Characters **\">"; 
        while ( $row = mysqli_fetch_array ( $result, MYSQL_ASSOC ) ) {
            print "<option value=\"".$row['id']."\">".$row['character_name']."</option>\r";    
        }
       }
                ?>
       </select>
                </form> 
            <?php
      } else {
       print "<h1>".$default_character_name."</h1>\n";
      }
            ?>  

 

<?php
/**
     * Get roster list
     *
     * @return object/NULL
     */
    function getRosterList()
    {
        $this->db->from('rosterList');
        $query = $this->db->get();
        if ($query->num_rows() > 0) 
        {
            return $query->result();
        }
        return null;
    }
    
    /**
     * Get list of roster by user ID
     *
     * @return object/NULL
     */
    function getRosterByUserID()
    {
        $this->db->from('rosterList');
        $this->db->where('userID', $userID);
        $query = $this->db->get();
        if ($query->num_rows() > 0) 
        {
            return $query->result();
        }
        return null;
    }  
?>

[Xtremer360]

*bump*

[PFMaBiSmAd]

In general, your queries should get the rows you want in the order that you want them, then you simply output the data the way you want when you iterate over the data. The two queries that only differ in the status_id value can be combined to get rows with status_id = 1 OR status_id = 2. You would then test the number of status_id values to determine what your code should do. See the following block of replacement code -

 

<?php
// get both active and inactive characters
if ($access_level_id == 2 || $access_level_id == 3) {
	$query = "SELECT
		characters.id,
		characters.character_name
		characters.status_id
		FROM
		characters
		WHERE
		characters.id <> '".$default_character_id."' AND
		characters.status_id IN(1,2)
		ORDER BY
		characters.character_name";
} else {
	$query = "SELECT
		characters.id,
		characters.character_name
		characters.status_id
		FROM
		characters
		INNER JOIN
		user_characters
		ON characters.id = user_characters.character_id
		INNER JOIN
		user_accounts
		ON user_accounts.id = user_characters.user_id
		WHERE
		user_accounts.id = '".$user_id."' AND
		user_characters.character_id <> '".$default_character_id."' AND
		characters.status_id IN(1,2)
		ORDER BY
		characters.character_name";
}
$result = mysqli_query ($dbc,$query);
$data[1] = array(); // active character data
$data[2] = array(); // inactive character data
while ( $row = mysqli_fetch_array ( $result, MYSQL_ASSOC ) ) {
	$data[$row['status_id']][] = $row;
}
// process active characters
if (count($data[1])) {
	if ($access_level_id == 2 || $access_level_id == 3) {
		print "<optgroup label=\"** Active Characters **\">"; // only show if there are status_id = 1 rows and if level 2 or 3
	}
	foreach($data[1] as $row){
		print "<option value=\"".$row['id']."\">".$row['character_name']."</option>\r"; // show all status_id = 1 options
	}
}
// process inactive characters
if (count($data[2])) {
	print "<optgroup label=\"** Inactive Characters **\">"; // show if any inactive characters
	foreach($data[2] as $row) {
		print "<option value=\"".$row['id']."\">".$row['character_name']."</option>\r"; // show all status_id = 2 options
	}
}

 

If the status_id will only ever have a 1 or 2 for a value, you can remove the characters.status_id IN(1,2) from both queries.

[Xtremer360]

Now I have to figure out how to work with that in my library and model for my CodeIgniter project.

[Xtremer360]

If user role is a user or editor then either display their default character or if more than one character then display dropdown of all handled characters. If user role is an administrator or webmaster then display their default character or ALL characters.

 

Displays Active (status-1), Inactive(status-2), Injured(status-3, Alumni(status-4) in separate option groups

 

controller:

$this->data['userRoster'] = $this->kowauth->getRosterList($this->data['userData']->usersRolesID);

 

library:

 

/**
* Get roster list
*
* @param   integer
* @return  object/NULL
*/
function getRosterList($usersRolesID)
{
    if (($usersRolesID == 4) || ($usersRolesID == 5))
    {
        return $this->ci->users->getAllRoster();
    } 
    else
    {
        return $this->ci->users->getRosterByUserID($this->ci->session->userdata('usersID'));
    }
}

 

Model:

 

/**
* Get roster list
*
* @return  object/NULL
*/
function getAllRoster()
{       
    $this->db->select('rosterName');
    $this->db->from('rosterList');
    $this->db->order_by('rosterName');
    $query = $this->db->get();
    if ($query->num_rows() > 0) 
    {
        return $query->result();
    }
    return null;
}

/**
* Get list of roster by user ID
*
* @return  object/NULL
*/
function getRosterByUserID($usersID)
{
    $this->db->select('rosterName');
    $this->db->from('rosterList');
    $this->db->where('userID', $usersID);
    $this->db->order_by('rosterName');
    $query = $this->db->get();
    if ($query->num_rows() > 0) 
    {
        return $query->result();
    }
    return null;
}

 

[Xtremer360]

What's supposed to happen is after the user logs in it has the default character id and the role id of the user that is held in the userData array. It runs the library function getRosterList. Inside that function it checks to see if the user has a role id of 4(admin) or 5(superadmin) and if they are then what I want it to do is get ALL the roster members which would include their default character and have it as the selected option. If they are not one of those two roles then I just want it to get the roster members that they control and have the preselected option as the default character id. And if they only have one character then it displays a h1 tag instead of the dropdown.

 

Here's also my view file.

 

<?php 
        echo '<pre>';
        print_r($userRoster);
        echo '</pre>';
        if (count($userRoster) == 1)
        {
            echo '<h1>'.$userRoster->rosterName.'</h1>';
        }
        else 
        {
            $options = array (
                $userRoster['id'] => $userRoster->rosterName
            );
            echo form_dropdown('userCharacters', $options, '', 'id="userCharacter"');
        }
        ?>

[Xtremer360]

Anybody have any ideas?