Пожалуйста оцените мое убогое ООП?

<?php

Class MyParser{	

	function __construct(){
		$this->regxp = '/(^\d+.\d+.\d+.\d+)\s*- -\s*([[]\d+\/\S+\s*[-|+]\d+])\s*"((POST|GET) \S+\s*\S+)"\s(\d+)\s*(\d+|-)\s*(\d+ \d+)?\s*"(http:\/\/\S+)"\s*"(\S+)\s*\(((compatible;\s*\S+ \d+.\d;))?(\s*\S+\s*\S+\s*\d+.\d+; \S+\s*\S+)\s*(\(\S+\s*\S+\s*\S+)?(\s*\D+)?/';
		$this->path = 'http_access.log';
	}

	// Читаем файл и достаем строки из $this->path
	function read_file()
	{
		if($handle = fopen($this->path, "r")){
			while(!feof($handle)) {
				yield trim(fgets($handle));
			}

			fclose($handle);
		}
	}

	// Получаем интересующие нас значения файла с помощью регулярки
	function get_log_array()
	{
		$iterator = $this->read_file();
		
		foreach ($iterator as $v => $k) {
			if(preg_match($this->regxp, $k, $m)){
				$ips[] 			= $m[1];
				$traffic[]  	= $m[6];
				$statusCode[] 	= $m[5];
				$urls[]         = $m[12];

				$main = array(
					'ips'        => $ips,
					'traffic'    => $traffic,
					'statusCode' => $statusCode,
					'urls'       => $urls,
				);
			}
		}
		return $main;
	}

	// Получаем кол-во ip
	function get_views()
	{
		return count($this->get_log_array()['ips']);	
	}

	// Получаем трафик 
	function get_traffic()
	{
		$traffic_arr = $this->get_log_array()['traffic'];
		for ($i = 0; $i < $this->get_views(); $i++) { 
			$traffic += (int)$traffic_arr[$i];
		}
		return $traffic;
	}

	// Получаем кол-во уникальных url
	function get_urls(){
		$urls = $this->get_log_array()['urls'];
		for ($i = 0; $i < $this->get_views(); $i++) { 
			$urls[] .= $urls[$i];
		}
		return count(array_unique($urls));
	}

	// получаем статус-код
	function get_status_codes(){
		$status_codes = $this->get_log_array()['statusCode'];
		$a = $b = 0;
		for ($i = 0; $i < count($status_codes); $i++) {
			if($status_codes[$i] == '200'){
				$a++;
			}
			elseif($status_codes[$i] == '301'){
				$b++;
			}
		}
		$sc = array(
			'200' => $a,
			'301' => $b,
		);

		return $sc;
	}


	// Формируем массив и json файл с этим массивом
	function create_json(){
		$file = file_get_contents('result.json');
		$log = json_encode($file, TRUE);
		unset($file);
		$log = array(
			'views' => $this->get_views(),
			'urls'	=> $this->get_urls(),
			'traffic' => $this->get_traffic(),
			'crawlers' => [
				'Google' => 2,
				'Bing' => 0,
				'Baidu' => 0,
				'Yandex' => 0,
			],
			'statusCodes' => $this->get_status_codes('http_access.log'),
		);
		file_put_contents('result.json', json_encode($log, JSON_PRETTY_PRINT));
		unset($log);
	}
}

$obj = new MyParser();
$obj->create_json();


Суть в том что мне дали задание распарсить файл в котором такие строки:
84.242.208.111- - [11/May/2013:06:31:00 +0200] "POST /chat.php HTTP/1.1" 200 354 "bim-bom.ru" "Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0"
это одна строка.

Нужно было достать из строки интересующую информацию и отправить ее в json файл.
{
    "views": 16,
    "urls": 4,
    "traffic": 187990,
    "crawlers": {
        "Google": 2,
        "Bing": 0,
        "Baidu": 0,
        "Yandex": 0
    },
    "statusCodes": {
        "200": 14,
        "301": 2
    }
}


Я сделал как просили, все работает. Но мне почему-то отказали с этим решением. Очень хотелось бы узнать почему.
Вообщем нужна критика кода и ооп)
  • Вопрос задан
  • 2345 просмотров
Решения вопроса 1
Stasgar
@Stasgar
Обученная макака
Во-первых: начните изучать архитектурную часть программирования, изучите паттерны проектирования, изучите SOLID, DRY, KISS и остальные модные словечки, постарайтесь всё это осознать, или, на крайняк - зазубрить. Всё придет с опытом, изначально все не понимали зачем всё так сложно, но эта сложность обусловлена неисчислимыми литрами слёз и потраченных нервов, всё не просто так.

Судя по всему это тестовое или учебное задание. От вас требовалось отоверинжинирить простую задачу. Давайте попробуем:

Суть задачи - есть файл с определенной структурой хранения данных, структура строковая. Требуется этот файл преобразовать в другую структуру данных и вывести эту структуру в json формате. Задача ясна.

Разобъем задачу на отдельные независимые этапы:
1) Преобразование одной структуры данных (текстового файла) в другую (объект, понятный PHP, к примеру)
2) Преобразование этой структуры данных в Json формат.
Первый вопрос, который может возникнуть - почему сразу не преобразовать в json? Ответ - при расширении системы в будущем - нам понадобится вывести данные в виде массива, или в виде XML, или даже в виде готового файла Excel. Нам будет сложно дополнять логику изначального класса, ничего при этом не сломав и не затронув уже существующий функционал. Также ответом на этот вопрос может являться каждая буква из SOLID принципов, подробнее отвечу дальше, когда буду пояснять за реализацию, см. ниже

Теперь рассмотрим эту задачу с точки зрения ООП, начнем думать не от конкретной реализации, а от интерфейса и абстракции (мы не парсим конкретный файл, мы парсим просто файл, мы не переводим его в конкретное представление json, мы переводим его просто в представление):
Нам понадобится 2 класса - непосредственно класс, читающий файл и преобразующий его в простейший тип данных (например PHP array). Второй класс - преобразователь простейшего типа данных парсера в какой-то определенный тип:
  1. LogFileReaded implements/extends FileReaderContract(интерфейс, возможно абстрактный класс, если понадобится предустановленная логика)

    Непосредственно наш преобразователь (не забываем, что передавать путь к файлу и все настройки нужно из вне, к примеру, в конструктор. Нельзя, как вы - хардкодить всё внутрь файла, на крайний случай - можно использовать конфигурацию), на ввод получает файл определенного типа с определенной известной структурой, на выходе выдает информацию из файла, преобразованную в базовый тип (массив, в нашем случае). Если нам когда-то понадобится парсить другой тип данных или структура данных будет изменена - мы сможем написать отдельный класс для этой логики, а не рушить целостность уже работающего кода, добавляя туда новую логику. Запомните - нужно стремиться не менять, а дополнять.

  2. JsonPresenter implements/extends DataTypePresenterContract

    Абстракция содержит контракт на метод output(), а в конструкторе принимются исходные данные. В конкретной реализации JsonPresenter в output() будет банальный json_encode() (да, это нормально, нет, класс не лишний и нет, json_encode() нельзя пихать в сам парсер) А теперь к вопросу - почему не следует просто запихать это всё в парсер и вместо массива отдать json: в будущем, когда система будет расширяться - нам понадобится представить данные в виде XML - что тогда будем делать - переписывать весь код парсера ради добавления switch case "json" и т.д.? А если что-то сломается во всей системе? А если вариантов представления станет настолько много, что файл будет просто не читаем? А при данном подходе достаточно будет просто написать новый класс XMLPresenter, или даже ExcelPresenter, который на выводе не строку будет выдавать, а целый файл (опустим типизацию output пока)). Также этот класс можно реализовать в виде декоратора (паттерн), да и много еще как.



В итоге у нас получается вполне сносная и, что важнее - расширяемая(в разумных пределах) архитектура, не забываем прописать класс клиента и определить конкретные реализации в одном месте - можно упороться и дописать сюда контейнер зависимостей.

К примеру: в итоге, если вас уже повысили, и вы вместо парсинга стали заниматься более высшими материями - новому программисту, чтобы дописать логику преобразования данных в Excel не нужно знать как конкретно вы преобразовывали когда-то эти данные в json, ему не нужно дебажить ваш код, ему достаточно посмотреть на интерфейс - отнаследоваться от него и написать свой собственный метод преобразования и дальше использовать его в нужном месте.

P.S. В данной реализации опускаются и упрощаются некоторые моменты для понятности
Ответ написан
Пригласить эксперта
Ответы на вопрос 2
Adamos
@Adamos
Во-первых, трудно поверить, что нет кучи готовых решений, разбирающих лог Апача.
Так что задание, очевидно, учебное, на использование языка и понимание, что такое ООП.
Так вот, ООП в РНР - это чтобы один раз сделать грязную работу, и больше в нее не заглядывать, используя готовый и по возможности очевидный интерфейс класса.
У вас же одноразовая портянка, в которой даже имена файлов жестко прописаны в коде, убогие комментарии вместо PHPDoc и вообще ощущение, что ООП вы начали заниматься вчера и считаете его просто возможностью загнать побольше функций в один класс.
Ну, и результат соответствующий. Вам нужно не исправить это решение, вам нужно позаниматься ООП в РНР некоторое время и прийти к соответствующей парадигме в мышлении. А этот класс можете просто выкинуть.
Ответ написан
Тут "куча" независимых задач:
1. Чтение файла (лога) построчно.
2. Разбор строки по правилам.
3. Сохранение результата разбора строки в каком-то промежуточном виде.
4. Сохранение в нужном текстовом формате.
5. Сохранение текста в файл.
6. Сборка всех задач в один простой алгоритм.
Все их лучше решить отдельно.
Ответ написан
Комментировать
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы
Depcon Екатеринбург
от 140 000 до 190 000 ₽
от 120 000 до 200 000 ₽
от 220 000 до 300 000 ₽
21 янв. 2022, в 15:28
6000 руб./за проект
21 янв. 2022, в 14:56
25000 руб./за проект
21 янв. 2022, в 14:29
10000 руб./за проект