if || or === 12строк||2строки :что выбрать?

[JSergRu]

if (isset($_GET['module']))
{	
	$module=$_GET['module'];
	$file="modules/".$module.".php";
		if (file_exists($file))
		{
			include $file;
		}else{ //обработка ошибки
		}
}else{
include ("modules/Index.php");
}

Вышенаписанный код сократил до двух строк

(@$module=$_GET['module']) or ($module="Index");
(@include "modules/".$module.".php") or die("Error???");

Вопрос: как это выглядит? Является ли это говнокодом? Что набрать в поисковике чтобы почитать о (/*выражение*/)or(/*выражение*/);? Есть ли другие комментарии и предложения?

 

[Breeze]

ужасно выглядит
уж лучше как-то так

$allowed_modules = Array(
	'index'=>1,
	'page1'=>1,
	'page2'=>1
);

$default_module = 'index';

$request_mod = isset($_GET['module']) ? $_GET['module'] : $default_module;

if ( !isset($allowed_modules[$request_mod]) || $allowed_modules[$request_mod]!=1 ) {
	$request_mod = $default_module;
}

include '/path/to/modules/'.$request_mod.'.php';

 

[С.]

Оператором "or" нет смысла злопотреблять. Например

(@$module=$_GET['module']) or ($module="Index");

довольно коротко записывается и без @or'ных изысков

$module= isset($_GET['module']) ? $_GET['module'] : 'Index';

Вторую строку не рассматриваю, поскольку она неравнозначна соответстбуюшему коду сверху. В ней нет обработки ошибки.

 

[Redjik]

JSergRu
собаки намекают, что не очень что-то хорошо

 

[Breeze]

Вторую строку не рассматриваю, поскольку она неравнозначна соответстбуюшему коду сверху. В ней нет обработки ошибки.

в ней нет еще много чего

 

[JSergRu]

Спасибо всем, кто участвовал в теме, особая благодарность пользователям Breeze и С.! ))

 

[Vin-Diesel]

$module = isset($_GET['module']) ? $_GET['module'] : 'index';
if(file_exists($module))
 include "modules/".$module.".php";
else
 die('error');

?module=../mysql-config
?module=../../../etc/.htpasswd%00
и т.д.


и вообще можно так:

$module = isset($_GET['module']) ? $_GET['module'] : 'index';if(file_exists($module)) include "modules/".$module.".php";else die('error');

 

[Vin-Diesel]

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

я так пошутил
Хотя для автора судя по названию темы критично количество строк.

Я первые два года когда занимался PHP тоже гнался "за количеством", зря...

 

[weregod]

Perl-style вообще не перевариваю, нечитабельно.

$module = isset($_GET['module']) ? $_GET['module'] : 'index';

а ещё лучше

$module = $request->get('module', 'index');

 

[JSergRu]

?module=../mysql-config
?module=../../../etc/.htpasswd%00
и т.д.

такие запросы планирую исключить примерно так

$allowed_modules = Array(
	'index'=>1,
	'page1'=>1,
	'page2'=>1
);

if ( !isset($allowed_modules[$request_mod]) || $allowed_modules[$request_mod]!=1 ) {
	$request_mod = $default_module;
}

include '/path/to/modules/'.$request_mod.'.php';

я так пошутил
Хотя для автора судя по названию темы критично количество строк.

отчасти правда, но поправлю чуть: количество +читабельность - в первом после код слишком раздутый, который грамотно сократили )

а ещё лучше

$module = $request->get('module', 'index');

это намек на создание универсального класса обработки входящих значений? )

 

[Redjik]

это намек на создание универсального класса обработки входящих значений? )

это, то, что есть в 99 процентах нормальных фреймворков

 

[Vin-Diesel]

такие запросы планирую исключить примерно так...

Лучше свичем тогда, он по-моему красивее и понятнее:

...
switch($_GET['module']) {
 case 'users':
 case 'info':
 case 'faq':
  $module = $_GET['module'];
  break;
 default:
  $module = 'index';
}
include '/path/to/modules/'.$module.'.php';

 

[Redjik]

Лучше свичем тогда, он по-моему красивее и понятнее

За такие советы надо расстреливать.

такие запросы планирую исключить примерно так...

Да хороший вариант, посмотри фреймы с MVC, там примерно такой же принцип в бутстрапе заложен.

UPD. а, так этот пример Breeze написал =))) смотрю как то вменяемо очень... =)

 

[JSergRu]

Лучше свичем тогда, он по-моему красивее и понятнее:

Спорить не буду, однако массив дает мне возможность закрывать доступ к страницам с помощью ключей массивов "на уровне пользователя" ... )

{$allowed_UsersGroup = Array(
    'Гость'=>0,
    'Пользователь'=>1,
    'Местный'=>2,

    'Модератор'=>6,

    'Администратор'=>8,
    'Бог проекта'=>9
);
}
{$allowed_Modules = Array(
    'Index'=>0,
    'Join'=>0,
    'Admin'=>8,
    'page4users'=>1
);
}

 

[Breeze]

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

в данном случае свич -- выкрутас. он неуниверсален, неуправляем и зело говнокодист в 90% случаев.

 

[Breeze]

Спорить не буду, однако массив дает мне возможность закрывать доступ к страницам с помощью ключей массивов "на уровне пользователя" ... )

и дает "искаропки" возможность работы с хранилищами настроек

 

[AmdY]

switch это признак пахнущего кода, любой switch можно заменить на фабричный метод или аналог.

в данном случае, каждый новый модуль потянет изменения в switch. Вопрос - Нафига?

 

[Redjik]

Chushkin
мне один раз попался в руки код с подобной структурой - там было в районе 30 кейсов

 

[AmdY]

Chushkin
в данном случае switch ничуть не читабельнее варианта с массивом, даже проигрывает. Не говоря уже о том, что массив как минимум лучше расширвается и может формироваться динамически под разные роли, а вариант с кейсом захардкожен и не расширяем. Не говоря уже

p.s. После всех понтов и подобных советов, больше не комментируй в разделе работа, буду сразу чистить посты.

 

[JSergRu]

...Другое дело, что если список модулей вынести в настройки...

Это удобно. )
Раз уж затронули ООП, switch, как я понимаю, подходит только для процедурного стиля?..