0
\$\begingroup\$

The getDynamicTeacherCount function is generating number-based reports from the table that I have in database and the getDynamicPercentage function generates percentage for these reports; then I am calling the getReports function from the related controller and pass the getReport returned array to the Laravel View and making a HTML table from that array.

The main problem is that with too many parameters needed when calling the getDynamicTeacherCount and getDynamicPercentage functions, the functions unreadable and hard to change or maintain.

namespace App;

use Illuminate\Database\Eloquent\Model;

class GeneralCBE extends Model
{
    /**
     * The database table name used by this model.
     * @var String
     */
    protected $table = 'general_cbe';

    /**
     * The attributes that are guarded.
     * @var array
     */
    protected $guarded = ['id'];

    /**
     * The constant for all applicants.
     * @var Const
     */
    const ALL = '';

    /**
     * The constant for all active applicants.
     * @var Const
     */
    const ACTIVE = '1';

    /**
     * The constant for all Dative applicants.
     * @var Const
     */
    const DEACTIVATE = '2';

    /**
     * The constant for all male applicants.
     * @var Const
     */
    const MALE = '1';

    /**
     * The constant for all female applicants.
     * @var Const
     */
    const FEMALE = '2';

    /**
     * The constant for all rejected applicants.
     * @var Const
     */
    const REJECTED = '6';

    /**
     * The constant for all those who rejected by reason one.
     * @var Const
     */
    const REASONONE = '1'; /**

     *  The constant for all those who rejected by reason tow.
     * @var Const
     */
    const REASONTOW = '2';

    /*
     * the constant for decisions
     */
    private $GRANTED  = array('tier2' => '1', 'etier2' => '2', 'tier3' => '3', 'etier3' => '4', 'tier4' => '5');



    /**
     * @param string $cbe
     * @param string $decision
     * @param string $institution
     * @param string $province
     * @param string $district
     * @return array
     */
    public static function getReports($cbe = '', $decision = '', $institution = '', $province = '', $district = '')
    {
        return [

            'genallt'              => self::getDynamicTeacherCount($cbe, self::ALL, self::ALL, self::ALL, $province, $district, $institution),
            'general_m_c'          => self::getDynamicTeacherCount($cbe, self::ALL, self::ALL, self::MALE, $province, $district, $institution),
            'general_m_p'          => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::ALL, self::ALL, self::ALL, $province, $district, $institution), $cbe, self::ALL, self::ALL, self::MALE, $province, $district, $institution),
            'general_f_c'          => self::getDynamicTeacherCount($cbe, self::ALL, self::ALL, self::FEMALE, $province, $district, $institution),
            'general_f_p'          => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::ALL, self::ALL, self::ALL, $province, $district, $institution), $cbe, self::ALL, self::ALL, self::FEMALE, $province, $district, $institution),

            'rejected'             => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution),
            'rejected_male'        => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::MALE, $province, $district, $institution),
            'rejected_male_p'      => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution), $cbe, self::REJECTED, self::ALL, self::MALE, $province, $district, $institution),
            'rejected_female'      => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::FEMALE, $province, $district, $institution),
            'rejected_female_p'    => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution), $cbe, self::REJECTED, self::ALL, self::FEMALE, $province, $district, $institution),
            'rejected_percentage'  => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::ALL, self::ALL, self::ALL, $province, $district, $institution), $cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution),

            'rejected_reason1'     => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution, 'rejected_reason', '1'),
            'rejected_reason1_m_c' => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::MALE, $province, $district, $institution, 'rejected_reason', '1'),
            'rejected_reason1_m_p' => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution, 'rejected_reason', '1'), $cbe, self::REJECTED, self::ALL, self::MALE, $province, $district, $institution, 'rejected_reason', '1'),
            'rejected_reason1_f_c' => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::FEMALE, $province, $district, $institution, 'rejected_reason', '1'),
            'rejected_reason1_f_p' => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution, 'rejected_reason', '1'), $cbe, self::REJECTED, self::ALL, self::FEMALE, $province, $district, $institution, 'rejected_reason', '1'),
            'rejected_reason1_p'   => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution), $cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution, 'rejected_reason', '1'),

            'rejected_reason2'     => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution, 'rejected_reason', '2'),
            'rejected_reason2_m_c' => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::MALE, $province, $district, $institution, 'rejected_reason', '2'),
            'rejected_reason2_m_p' => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution, 'rejected_reason', '2'), $cbe, self::REJECTED, self::ALL, self::MALE, $province, $district, $institution, 'rejected_reason', '2'),
            'rejected_reason2_f_c' => self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::FEMALE, $province, $district, $institution, 'rejected_reason', '2'),
            'rejected_reason2_f_p' => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution, 'rejected_reason', '2'), $cbe, self::REJECTED, self::ALL, self::FEMALE, $province, $district, $institution, 'rejected_reason', '2'),
            'rejected_reason2_p'   => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution), $cbe, self::REJECTED, self::ALL, self::ALL, $province, $district, $institution, 'rejected_reason', '2'),


            'allt'                 => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution),

            'male'                 => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $district),
            'malep'                => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $district), $cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $district),
            'fmale'                => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $district),
            'fmalep'               => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $district), $cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $district),

            'edu_ttc'               => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '4'),
            'edu_ttc_p'             => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution), $cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '4'),
            'edu_ttc_male_c'        => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'level_of_edu', '4'),
            'edu_ttc_male_p'        => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '4'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'level_of_edu', '4'),
            'edu_ttc_fmale_c'       => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'level_of_edu', '4'),
            'edu_ttc_fmale_p'       => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '4'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'level_of_edu', '4'),

            'edu_ba'               => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '2'),
            'edu_ba_p'             => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution), $cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '2'),
            'edu_ba_male_c'        => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'level_of_edu', '2'),
            'edu_ba_male_p'        => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '2'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'level_of_edu', '2'),
            'edu_ba_fmale_c'       => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'level_of_edu', '2'),
            'edu_ba_fmale_p'       => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '2'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'level_of_edu', '2'),

            'edu_ma'               => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '3'),
            'edu_ma_p'             => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution), $cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '3'),
            'edu_ma_male_c'        => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'level_of_edu', '3'),
            'edu_ma_male_p'        => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '3'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'level_of_edu', '3'),
            'edu_ma_fmale_c'       => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'level_of_edu', '3'),
            'edu_ma_fmale_p'       => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'level_of_edu', '3'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'level_of_edu', '3'),



            'edu_prim_count'       => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '1'),
            'edu_prim_p'           => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '1'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '1'),
            'edu_prim_m_c'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_level', '1'),
            'edu_prim_m_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '1'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_level', '1'),
            'edu_prim_f_c'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '1'),
            'edu_prim_f_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '1'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '1'),

            'edu_sec_count'       => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '2'),
            'edu_sec_p'           => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '2'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '2'),
            'edu_sec_m_c'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_level', '2'),
            'edu_sec_m_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '2'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_level', '2'),
            'edu_sec_f_c'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '2'),
            'edu_sec_f_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '2'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '2'),

            'edu_uper_count'       => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '3'),
            'edu_uper_p'           => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '3'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '3'),
            'edu_uper_m_c'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_level', '3'),
            'edu_uper_m_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '3'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_level', '3'),
            'edu_uper_f_c'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '3'),
            'edu_uper_f_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_level', '3'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_level', '3'),

            'pashto_count'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '1'),
            'pashto_per'           => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '1'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '1'),
            'pashto_male_c'        => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_language', '1'),
            'pasto_male_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '1'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_language', '1'),
            'pashto_fmale_c'       => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '1'),
            'pasto_fmale_p'        => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '1'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '1'),

            'dari_count'           => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '2'),
            'dari_per'             => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '2'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '2'),
            'dari_male_c'          => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_language', '2'),
            'dari_male_p'          => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '2'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_language', '2'),
            'dari_fmale_c'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '2'),
            'dari_fmale_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '2'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '2'),

            'other_count'           => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '3'),
            'other_per'             => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '3'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '3'),
            'other_male_c'          => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_language', '3'),
            'other_male_p'          => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '3'),$cbe, $decision, self::ACTIVE, self::MALE, $province, $district, $institution, 'teaching_language', '3'),
            'other_fmale_c'         => self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '3'),
            'other_fmale_p'         => self::getDynamicPercentage(self::getDynamicTeacherCount($cbe, $decision, self::ACTIVE, self::ALL, $province, $district, $institution, 'teaching_language', '3'),$cbe, $decision, self::ACTIVE, self::FEMALE, $province, $district, $institution, 'teaching_language', '3')


        ];
    }


    /**
     * give you the percentage of counts that comes from db
     * @param $val
     * @param $all
     * @return float
     */
    public static function getPercentage($val, $all)
    {
        return round($val * 100 / $all);
    }

    /**
     * get all rejected teacher count
     * @return mixed
     */
    public static function getRejectedTeacherCount()
    {
        return self::where('decision', 3)->count();
    }

    /**
     * return number of active teacher
     * @return mixed
     */
    public static function getAllActiveTeacherCount()
    {
        return self::activeTeacherObj()->count();
    }

    /**
     * returns the dynamic count of teachers base on function arguments.
     * many staff is automated in this function for example get raided
     * of many where clauses and many if conditions
     * @param string $cbe
     * @param string $decision
     * @param string $active
     * @param string $gender
     * @param string $province
     * @param string $district
     * @param string $institution
     * @param string $additionFieldKey
     * @param string $additionFieldValue
     * @return mixed
     */
    public static function getDynamicTeacherCount($cbe = '', $decision = '', $active = '', $gender = '', $province = '', $district = '',
                                                  $institution = '', $additionFieldKey = '', $additionFieldValue = '')
    {
        $fields = [
            'tier'                      => $cbe,
            'decision'                  => $decision,
            'active'                    => $active,
            'gender'                    => $gender,
            'pro_province'              => $province,
            'pro_district'              => $district,
            'institution_where_studied' => $institution,
            $additionFieldKey           => $additionFieldValue
        ];

        $result = new self;

        foreach ($fields as $attr => $value)
        {
            if(! empty($value))
            {
                $result = $result->where($attr, $value);
            }
        }

        return number_format($result->count());
    }

    /**
     * returns the dynamic Percentage of teachers base on function arguments.
     * many staff is automated in this function for example get raided
     * of many where clauses and many if conditions
     * @param $yValue
     * @param string $cbe
     * @param string $decision
     * @param string $active
     * @param string $gender
     * @param string $province
     * @param string $district
     * @param string $institution
     * @param string $additionFieldOne
     * @param string $additionFieldTow
     * @return mixed
     */
    public static function getDynamicPercentage($yValue, $cbe = '', $decision = '', $active = '', $gender = '', $province = '', $district = '',
                                                $institution = '', $additionFieldOne = '', $additionFieldTow = '')
    {
        $result = self::getDynamicTeacherCount($cbe, $decision, $active, $gender, $province, $district,
                                                $institution, $additionFieldOne, $additionFieldTow);

        $intResult = intval(str_replace(',', '', $result));

        return self::getPercentage($intResult, intval(str_replace(',', '', $yValue)));
    }

    /**
     * returns all teachers that the are active
     * @return static
     */
    public static function activeTeacherObj()
    {
        return self::where('active', 1);
    }


}
\$\endgroup\$
2
  • \$\begingroup\$ Please take a moment to edit the title and body of your question to tell us a bit about what the code is doing. We all want to improve our code on this site so there is no need to ask for that in your title. \$\endgroup\$
    – Phrancis
    Commented Mar 24, 2016 at 16:27
  • 1
    \$\begingroup\$ community-challenge doesn't seem appropriate. I think this is not an implementation of any proposed community-challenge on this site. Accordingly I have removed the tag. Additionally you should include a description of what your code does to give context for the reviewers \$\endgroup\$
    – Vogel612
    Commented Mar 24, 2016 at 16:27

2 Answers 2

1
\$\begingroup\$

I see what your talking about there. That is way too many variables. After reviewing all of your methods, it seems the best solution might be to make an Object Class that would encapsulate the objects. Maybe Teacher? Obviously add whatever variables are required to the below class, I just put what I saw.

class Teacher {

    private $cbe;
    private $decision;
    private $active;
    private $gender;
    private $province;
    private $district;
    private $institution;

    public function __construct($array) {
        $this->cbe = $array['cbe'];
        $this->decision = $array['decision'];
        $this->active = $array['active'];
        $this->gender = $array['gender'];
        $this->province = $array['province'];
        $this->district = $array['district'];
        $this->institution = $array['institution'];
    }

//proper getters for each, no setters needed as this is used then thrown away

You could then rewrite the getDynamicTeacherCount() and getDynamicPercentage() to use a Teacher object instead. Your getReports() would then look something like this:

public static function getReports($teacher)
{
    return [

        'genallt'              => self::getDynamicTeacherCount($teacher),
        'general_m_c'          => self::getDynamicTeacherCount($teacher),
        'general_m_p'          => self::getDynamicPercentage(self::getDynamicTeacherCount($teacher), $teacher),
        'general_f_c'          =>  ......
\$\endgroup\$
1
\$\begingroup\$

First off, would want to know what made you do all this. As I only know code you come up for certain use case and not use case itself all I can do is refactor your code. There is high chance that you approached implementing functionality incorrectly.

With that being said here are some tips:

  1. Very rarely should you pass 3 arguments to a function. In your case we are talking about 9 parameters. I would not have anything against PHP throwing error in such cases.
  2. If you don't know when to use static functions, don't use them at all.
  3. Spelling

Now, let's address the elephant in the room. My way might not be the best but leaves you with almost 0 code. Everything is stored in db, including reports configuration. Below model is responsible for it. You can throw in field like $description to describe what given report shows.

class TeacherReport extends Model {

    public $name;
    public $decision;
    public $status;
    public $gender;
    public $additionalParameter;
    public $additionalValue;

}

Next object I have added is Teacher. You will be calling getReports() directly on this class.

class Teacher extends Model {

    public $tier;
    public $decision;
    public $active;
    public $gender;
    public $province;
    public $district;
    public $institution;
    public $additionFieldValue;

    const ALL_GENDERS = '';

    public function getReports() {
        $teacherReport = new TeacherReport();
        $result = [];
        foreach([$teacherReport] as $model) {
            $count = $this->getCount($teacherReport);
            $totalCount = $this->getTotalCount($teacherReport);
            $percentage = $this->getPercentage($count, $totalCount);
            $result[$model->name] = ['count' => $count, 'percentage' => $percentage];
        }
        return $result;
    }

    private function getCount(TeacherReport $teacherReport) {
        $attributes = $this->getMappedCountAttributes($teacherReport);
        $result = new Teacher();
        foreach ($attributes as $attr => $value) {
            if(!empty($value)) {
                $result = $result->where($attr, $value);
            }
        }
        return $result->count();
    }

    private function getMappedCountAttributes(TeacherReport $teacherReport) {
        return [
            'tier'                      => $this->tier,
            'decision'                  => $teacherReport->decision,
            'active'                    => $teacherReport->status,
            'gender'                    => $teacherReport->gender,
            'pro_province'              => $this->province,
            'pro_district'              => $this->district,
            'institution_where_studied' => $this->institution,
            $teacherReport->additionalParameter   => $teacherReport->additionalValue
        ];
    }

    private function getTotalCount(TeacherReport $teacherReport) {
        $gender = $teacherReport->gender;
        $teacherReport->gender = self::ALL_GENDERS;
        $count = $this->getCount($teacherReport);
        $teacherReport->gender = $gender;
        return $count;
    }

    private function getPercentage($part, $whole) {
        return round($part * 100 / $whole);
    }
}
\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.