Ryan Walkowski
Ryan Walkowski

Reputation: 361

Making code more efficient and less a copy and paste

I've been working on a piece of code that that pulls the name of a guild and with it the information of boss/monsters said guild has killed in an online game. There are many many monsters in this game and every one has three difficulty settings. I have managed to get the code to do what i want however it has an enormous amount of copy and paste and ive only done about 1/5 of the total amount of enteries. I really cant think how to make this code less of a giant bloat. This is the code for just one monster for the 3 difficulty settings as you can see it's alot just for one. there are probably another 60 of these!. Can anybody help me understand better ways to do this. Thanks!

$sql = 'SELECT * FROM `phpbb_profile_fields_data`';

  $result = $db->sql_query($sql);

  while ($row = $db->sql_fetchrow($result))
  {      
    /////////////////////////////////// START - 8 MAN BONETHRASHER
    $normal = '';
    $hard = '';
    $nightmare = '';
    /////// START - CHECK NORMAL
    if ($row['pf_kp_em_no_bonethr'] == '1')
        {
            $normal = '&nbsp;<img src="/styles/subsilver2/theme/images/soap/no.png" />';
        }       
    else if ($row['pf_kp_em_no_bonethr'] == '2')
        {
            $normal = '';
        }       
    else if (is_null($row['pf_kp_em_no_bonethr'])) 
        {
            echo "Boss was set as NULL This should not happen!";
        }
    else
        {
            echo "Sosia messed up go hit him in the face.";
        }
    /////// END - CHECK NORMAL  
    /////// START - CHECK HARD
    if ($row['pf_kp_em_ha_bonethr'] == '1')
        {
            $hard = '&nbsp;<img src="/styles/subsilver2/theme/images/soap/ha.png" />';
        }       
    else if ($row['pf_kp_em_ha_bonethr'] == '2')
        {
            $hard = '';
        }       
    else if (is_null($row['pf_kp_em_ha_bonethr'])) 
        {
            echo "Boss was set as NULL This should not happen!";
        }
    else
        {
            echo "Sosia messed up go hit him in the face.";
        }
    /////// END - CHECK HARD    
    /////// START - CHECK NIGHTMARE
    if ($row['pf_kp_em_kn_bonethr'] == '1')
        {
            $nightmare ='&nbsp;<img src="/styles/subsilver2/theme/images/soap/kn.png" />';
        }       
    else if ($row['pf_kp_em_kn_bonethr'] == '2')
        {
            $nightmare = '';
        }       
    else if (is_null($row['pf_kp_em_kn_bonethr'])) 
        {
            echo "Boss was set as NULL This should not happen!";
        }
    else
        {
            echo "Sosia messed up go hit him in the face.";
        }
    /////// END - CHECK NIGHTMARE   

if ($normal == '' && $hard == '' && $nightmare == '') 
        {

        }
    else
        {
            $template->assign_block_vars('8m_bonethrasher', array(
            'VAR1' => $row['pf_guild_name'],
            'VAR2' => $normal,
            'VAR3' => $hard,
            'VAR4' => $nightmare,
            ));
        }

  }

  $db->sql_freeresult($result);         

Upvotes: 0

Views: 113

Answers (2)

Shattuck
Shattuck

Reputation: 2780

I'm still slightly fuzzy at what you are trying to do, but I'll give helping you out a shot.

You could probably get away will creating a class that does all of this.

For example:

class checks {

    public function checkBosses($normalBoss, $hardBoss, $nightmareBoss) {

        $difficulties = array();

        $difficulties['normal'] = array('boss' => $normalBoss);
        $difficulties['hard'] = array('boss' => $hardBoss);
        $difficulties['nightmare'] = array('boss' => $nightmareBoss);

        foreach ($this->difficulties as $difficulty -> $boss) {
            $this->difficulties[$difficulty]['result'] = checkDifficulty($boss['boss'], $difficulty);
        }

        $normal = $this->difficulties['normal']['result'];
        $hard = $this->difficulties['hard']['result'];
        $nightmare = $this->difficulties['nightmare']['result'];


        if ($normal == '' && $hard == '' && $nightmare == '') {
            return null;
        } else {
            return array(
                'normal' => $normal,
                'hard' => $hard,
                'nightmare' => $nightmare,
            );
        }
    }

    protected function checkDifficulty($boss, $difficulty) {

        if ($difficulty == 'normal') {
            $image = '&nbsp;<img src="/styles/subsilver2/theme/images/soap/no.png" />';
        } else if ($difficulty == 'hard') {
            $image = '&nbsp;<img src="/styles/subsilver2/theme/images/soap/ha.png" />';
        } else if ($difficulty == 'nightmare') {
            $image = '&nbsp;<img src="/styles/subsilver2/theme/images/soap/kn.png" />';
        } 

        if ($boss == '1') {
            return $image;
        } else if ($boss == '2') {
            return '';
        } else if (is_null($boss)) {
            echo "Boss was set as NULL This should not happen!";
        } else {
            echo "Sosia messed up go hit him in the face.";
        }
    }
}

Then all you would need to do is call:

$checkResult = checks::checkBosses($row['pf_kp_em_no_bonethr'], $row['pf_kp_em_ha_bonethr'], $row['pf_kp_em_kn_bonethr']);

if ($checkResult != null) {
    $template->assign_block_vars('8m_bonethrasher', array(
        'VAR1' => $row['pf_guild_name'],
        'VAR2' => $normal,
        'VAR3' => $hard,
        'VAR4' => $nightmare,
    ));
}

Upvotes: 1

Keith Frey
Keith Frey

Reputation: 691

If you can retrieve an array of bosses, you can do a foreach loop on them to run that same bit of code for each boss like this:

foreach ($bosses as $boss) {
    //Full code to be repeated for each boss here
}

Upvotes: 0

Related Questions