I ask for constructive criticism on my PHP code

Question

PHP

Greetings! It so happened that I improve my programming skills completely on my own, and due to the specifics of the work, no one checks my code. Therefore, I ask you to criticize me so that I can sort out the mistakes and work on them. I understand that reading someone else's, to put it mildly, not very good code takes a lot of time, but still I hope that there will be well-wishers who will point out my mistakes to me and improve the quality of my code.
For a long time I thought about whether this question is appropriate on this resource, after reading the question , I decided to publish it.
Some system that I'm writing now.
A small test task that I recently did.

Answer 1

I remember a few years ago I was also approached, let's call him Gennady, who "recruited cool freelancers in a new cool studio" and, apparently, in order for me to prove my "coolness", sent a task (more like a real TK), according to which it was necessary to implement a full-fledged a real estate guide with all the functionality, in the letter attaching a link to a half-gigabyte dump of either CSV or SQL databases. Moreover, I had to write it immediately and in 12 hours, every 3 hours reporting to him personally on the work done.
When Gennady was refused, he was very offended, calling me all sorts of words, such as “incompetent”, “lazy”, “missing his chance”, etc. on which we said goodbye.
Just forget it and never again get fooled by “test tasks” of more than 100 lines, and even more so, including the development of a finished product. In a couple of years, you will tell your descendants the same way :) Do you
want to test my skill as a programmer? 100-200 lines of a good assignment is enough for this.
Want to just look at the quality of my code and my skills as a designer of large systems? Welcome to my github, where there are a couple of my projects, or ask to "sketch on a piece of paper" something else.
For some reason, at any interview in any large company, they ask either to throw in the simplest implementation, or they give them a small piece to write at home, and all these Gennady rogues are immediately given an “Online store” or “Real Estate Directory” with full functionality.

Answer 2

if( defined('DEBUG') && DEBUG )

    $logger->printrLog();

That's right, you don't have to do it. Beg.

Answer 3

1. Global variables are bad... about why a lot of things are badly written.

$db     = Db::initDb($dbConf['host'], $dbConf['user'], $dbConf['pass'], $dbConf['db'] );
$db->query('SET NAMES UTF8');
$page   = UserPage::initPage();
$logger = Logger::initLogger();
$widget = WidgetLoader::initWidgetLoader();
$tpl    = Template::initTemplate(TEMPLATES_PATH);

2. Write comments on class methods next to the method, not in the class description.

Answer 4

implode just to remove crutches like $sql = substr($sql, 0, strlen($sql) - 2);
Habril you :) Previously, my code was exactly the same

Answer 5

- Use the DIRECTORY_SEPARATOR constant.
- Global variables are sad, they violate design patterns, creating crutches, I use them exclusively for screwing a temporary crutch, because it’s pressed, but I need to do it, then I refactor through patterns.
- Db:: initDb - why use a factory in this case ??? We are creating an object, that is, it should be new DB (); purely in terms of OOP. And by the way, if your classes do not have a completely special functionality, then you do not need to wrap standard functions at all. For example, I have a database class that abstracts physical connections to the DBMS, and switching between databases in one DBMS. It is accessed by different objects indicating only the name of the “gateway” (I called it that), and the class already guarantees that the request will be made according to the required DBMS + DB. Once again, in your case, I see no reason to use a factory (I use a factory to simply create objects with a built-in "gateway").
I advise you to do this at the beginning of initialization:
// directory with the system
define('DIR_ROOT', __DIR__.DIRECTORY_SEPARATOR); // kernel root.
// directory with engine code
define('DIR_CORE', DIR_ROOT.'core'.DIRECTORY_SEPARATOR);
// directory with engine interfaces
define('DIR_CORE_INTERFACES', DIR_CORE.'interfaces'.DIRECTORY_SEPARATOR);
========
I can write a whole post here on my production culture, but have pity on me ;-) I hope I helped.
// directory with engine exceptions
define('DIR_CORE_EXCEPTIONS', DIR_CORE.'exceptions'.DIRECTORY_SEPARATOR);
// directory with abstract engine modules
define('DIR_CORE_ABSTRACT', DIR_CORE.'abstract-modules'.DIRECTORY_SEPARATOR);

Answer 6

The code is teeming with dead code, like

 throw new Exception('Template file '.$templateLink.' not found!');
if( defined('DEBUG') && DEBUG )
  $this->html = 'Template file '.$templateLink.' not found!';
else
  $this->html = '';
return false;

After an exception, the code will never be executed.

Answer 7

Comment the code. Even if for yourself. Will help in the future.

Answer 8

Assigned :
The ApacheLogAnalyzer class has too many responsibilities and hard links:
— reads data from the log (albeit through ApacheLogReader, but still);
- analyzes them;
- writes them to the database (albeit through MysqlLogDataWriter, but still)
There is no obvious need to instantiate ApacheLogReader and MysqlLogDataWriter in ApacheLogAnalyzer. I also do not see the need to instantiate mysqli in MysqlLogDataWriter, and in general PDO is better, as indicated above.
You can do without strong refactoring something like

$reader = new ApacheLogReader('access.log'); 
$db = new PDO('mysql:dbname=fl;host=localhost', 'fl', 'fl'); // @todo Обработка ошибок
$writer = new PDOLogDataWriter($db); 
$analyzer = new LogAnalyzer($reader, $writer);
...

So we get a more flexible and easier to test architecture with little or no overhead and big changes. And given the specifics (the reader and writer methods are called only in one place in the analyzer), you can do

$reader = new ApacheLogReader('access.log'); 
$log_elems = $reader->getLogElems();
$analyzer = new LogAnalyzer($log_elems);
$data = $analyzer->getAnalyzedData();
$db = new PDO('mysql:dbname=fl;host=localhost', 'fl', 'fl'); // @todo Обработка ошибок
$writer = new PDOLogDataWriter($db); 
$writer->write($data);

by generally getting rid of these dependencies in the analyzer and making it even easier to test, but also without global changes.
You can go even further, implement a bunch of interfaces and patterns, and make the analyzer completely independent of the data source and their format, format, not even from the method of analysis, but this will probably already be “OOP of the brain” and premature abstraction :)

Answer 9

About the test task. I think that it was necessary to analyze the log and shove it into the database.
I see problems with the algorithm: count everything, write everything (also in one request). There are a couple of problems at once: with a large log, it will not be enough; upon re-analysis, all data will be inserted into the log again.