Задать вопрос
@v1ad13r
php

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

<?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
    }
}


Я сделал как просили, все работает. Но мне почему-то отказали с этим решением. Очень хотелось бы узнать почему.
Вообщем нужна критика кода и ооп)
  • Вопрос задан
  • 2394 просмотра
Комментировать
Подписаться 11 Простой Комментировать
Решения вопроса 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. В данной реализации опускаются и упрощаются некоторые моменты для понятности
Ответ написан
21 комментарий
21 комментарий
Пригласить эксперта
Ответы на вопрос 2
Adamos
@Adamos
Во-первых, трудно поверить, что нет кучи готовых решений, разбирающих лог Апача.
Так что задание, очевидно, учебное, на использование языка и понимание, что такое ООП.
Так вот, ООП в РНР - это чтобы один раз сделать грязную работу, и больше в нее не заглядывать, используя готовый и по возможности очевидный интерфейс класса.
У вас же одноразовая портянка, в которой даже имена файлов жестко прописаны в коде, убогие комментарии вместо PHPDoc и вообще ощущение, что ООП вы начали заниматься вчера и считаете его просто возможностью загнать побольше функций в один класс.
Ну, и результат соответствующий. Вам нужно не исправить это решение, вам нужно позаниматься ООП в РНР некоторое время и прийти к соответствующей парадигме в мышлении. А этот класс можете просто выкинуть.
Ответ написан
4 комментария
4 комментария
@fpinger
Тут "куча" независимых задач:
1. Чтение файла (лога) построчно.
2. Разбор строки по правилам.
3. Сохранение результата разбора строки в каком-то промежуточном виде.
4. Сохранение в нужном текстовом формате.
5. Сохранение текста в файл.
6. Сборка всех задач в один простой алгоритм.
Все их лучше решить отдельно.
Ответ написан
Комментировать
Комментировать
Ваш ответ на вопрос

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

Похожие вопросы
Вакансии с Хабр Карьеры
PHP разработчик
Lachestry • Таганрог
от 170 000 до 200 000 ₽
PHP разработчик Junior+/Middle
amoCRM • Москва
от 130 000 ₽
Бэкенд разработчик (PHP Symfony)
Продумáн • Москва
До 260 000 ₽
Ещё вакансии
Заказы с Хабр Фриланса
Доработать интеграцию на сайте в Tilda через вебхук
25 дек. 2024, в 06:40
3000 руб./за проект
Периодически возникает ошибка при включении сайта на open server
25 дек. 2024, в 02:45
1000 руб./за проект
Разработать нативное приложение под iOS
25 дек. 2024, в 00:52
150000 руб./за проект
Ещё заказы