abraham foto abraham foto - 9 days ago 5
PHP Question

PHP: breaking a function to functions with a single task?

I have a function that performs about 5 tasks at the same time that is strictly against the principles of OOP. can anyone help me to break it dawn? specially calling a function in other function is a kind of tricky for me.

public function status(){
$client = new Client();
$notification=Notification::where('active',1)->get();
$status = Status::where('name', 'health')->first();
$default_frequency = 1;

foreach ($notification as $note) {
$status_health = $note->status('health');

$check_frequency = isset($note->check_frequency) ? intval($note->check_frequency) : $default_frequency;

$date = \Carbon\Carbon::parse($status_health['timestamp']);
$elapsed_time = $date->diffInMinutes();

if($elapsed_time < $check_frequency){
continue;
}

$response = $client->get($note->website_url, ['http_errors' => false]);
$resCode = $response->getStatusCode();

$note->statuses()->attach($status,['values'=> $resCode === 200 ? 'up' : 'down']);
}
}

Answer

I don't think you need to refine the code too much. If I understood correctly, you're trying to update the status of your notifications within some specified intervals.

This could be break into two tasks:

  1. Getting all notifications and loop through each one of them
  2. Update the status of each notification based on some criteria

Here is my attempt to refine and simplify your code a little:

Class YourClass {

    const DEFAULT_FREQUENCY = 1;

    private $client;

    public function __construct(Client $clinet)
    {
        $this->client = $clinet;
    }

    public function status()
    {
        $notifications = Notification::where('active', 1)->get();
        $status = Status::where('name', 'health')->first();

        foreach ($notification as $notification) {
            $this->updateStatus($notification, $status);
        }
    }

    private function updateStatus(Notification $notification, Status $status)
    {
        $status_health = $notification->status('health');

        $frequency = $this->getFrequency($notification);

        $elapsed_time = \Carbon\Carbon::parse($status_health['timestamp'])->diffInMinutes();

        if ($elapsed_time >= $frequency) {
            $response = $this->client->get($notification->website_url, ['http_errors' => false]);   
            $notification->statuses()->attach($status, [
                'values'=> $response->getStatusCode() === 200 ? 'up' : 'down'
            ]);
        }
    }

    private function getFrequency(Notification $notification)
    {
        return isset($notification->check_frequency) 
            ? intval($notification->check_frequency) 
            : self::DEFAULT_FREQUENCY;
    }
}
Comments