In pursuit of execution speed and lower memory consumption, I wrote a script (an analog of cron-a).

I want to ask you to look at this piece of code and write what can be improved and how to get rid of.

Here is the code :

<?php date_default_timezone_set('Europe/Moscow'); define ("CONST_TIME",1);// in second class croner { function __construct() { $this->start_on = time(); } public function GetTime($i) { if(is_numeric($i)){ $time = $i + $this->task->$i->last; if(time() >= $time) { $this->task->$i->last = time(); return TRUE; } } if(strpos($i,':') !== false){ // and is_numeric(str_replace(':','',$i)) // if($this->task->$i->next == 0){ if(date('H:i') <= $i){ $this->task->$i->last = time(); $this->task->$i->next = time() + GetTimeAtr('d'); return TRUE; } }else{ if($this->task->$i->next >= time()){ $this->task->$i->last = time(); $this->task->$i->next = time() + GetTimeAtr('d'); return TRUE; } } } return FALSE; } public function GetTimeStart() { return $this->start_on; } public function AddTaskClock($time,$f,$p,$name) { $this->task->$time->start = time();// время начала,старта таска $this->task->$time->last = time(); // время последнего вызова таска $this->task->$time->func = $f; // функция вызова $this->task->$time->param = $p; // параметры для ф. вызова $this->task->$time->taskname = $name;// имя таска $this->task->$time->next = 0; // время следующего выполнения return true; } public function FindTask() { foreach($this->task as $key => $value) { if($this->GetTime($key)){ $this->CalledFunc($key); } } } public function CalledFunc( $i ) { return call_user_func( $this->task->$i->func, $this->task->$i->param ); } private function GetTimeAtr( $a ) { switch ($a) { case 'm' :$r = 60; break; case 'h' :$r = 3600; break; case 'd' :$r = 86400; break; case 'w' :$r = 604800; break; case 'M' :$r = 2629743; break; case 'y' :$r = 31556926; break; } return $r; } public function StartCron(){ while( TRUE ) { $this->FindTask(); sleep(CONST_TIME); } } } ?>
  • On this site there is a special section called Code Review. Here there also address with the question. - Vlad from Moscow
  • one
    OOP, execution speed and memory consumption are unrelated. Moreover, OOP is more likely to draw down these parameters compared to well-written procedural code. The benefits of OOP are completely different. - rjhdby
  • To learn OOP you need to program in a language in which there is nothing but classes. So that there is no opportunity to slip into the procedure. And what a lot of superfluous - so do not worry. The next time you make another program on a smaller one. If it works, then figs with it. There is nothing to lose, you have to blow ahead. - Sergey
  • @Sergey disagree. In order to learn OOP, you need to be put for a beautiful OOP project for rework. A bit of potrekkodish, a bit of kick you, and learn. And in any "pure-OOP" language, you can write a singleton on 100 methods, which will obviously not add to understanding. - Goncharov Alexander

1 answer 1

Short Review:

  1. No compliance with PSR standards (name of functions, variables)
  2. Ambiguity in the operation of the function (multiple exit points from the function return TRUE or FALSE)
  3. The $ task property is clearly not definitive, is essentially an array, but for some reason it is implemented as an object, and this allows me to access it through $ croner-> task and make changes ( Violation of the OCP principle in SOLID )
  4. There is no documentation of the parameters; I cannot understand what I need to transmit without opening the code.
  5. Some functions that are systemic are made public.
  6. Some functions accept both a string and a number, the ambiguity of parameters.
  7. Violation of the DRY principle ( do not repeat ) in the $ this-> task -> $ i-> last, $ this-> task -> $ i-> next lines of the GetTime function
  8. A more adequate solution was to make an array of tasks, where to store elements of a thread of the Task class.
  9. Thoughts: “I don’t understand why the GetTime function doesn't return anything to me, I explicitly called the GetTime function, it says get, and why didn’t it return anything to me?” Problems in reflecting the functionality in the title.

In general, these libraries probably have on github, and it is easier to take already written functionality than to write your bikes.

  • Thanks for the criticism. - Jared J