Вадим Т.
3240 сообщений
#17 лет назад
Насчет кода. Да, код ужасный. Мешанина HTML и PHP, логики и представления, без четкой структуры и с явными уязвимостями. Но не факт что разработчик совсем уж виноват. Возможно он использовал предоставленный ему заказчиком код для последующей доработки. И возможно бюджет был всего лишь на оплату 0.5 человеко-часа.

Я сам сторонник того, чтобы писать идеальный образцовый well documented код без избыточности, дублирования логики и ламерства. Но если приходит заказ встроить например некий модуль в OsCommerce? Не стану же я переделывать более 1000 PHP скриптов, написанных много лет назад десятками разработчиков OsCommerce. Или если поручат дописывать функционал под CnCat... В общем, код спеца можно оценивать только если это именно им написанный с нуля код, и было выделено достаточно времени на разработку.
 А.
44 сообщения
#17 лет назад
Цитата ("sxq"):

ArtLab, вы заказчик? вы так уверено говорите!

Мы работали С ArtLab и работаем , поэтому у них есть такая информация.

Я не могу здесь назвать конкретную сумму, скажу, что она намного больше Вашего примера. Только за проект SiteMarket.
Михаил П.
6 сообщений
#17 лет назад
Цитата ("tvv"):
Насчет кода. Да, код ужасный. Мешанина HTML и PHP, логики и представления, без четкой структуры и с явными уязвимостями. Но не факт что разработчик совсем уж виноват. Возможно он использовал предоставленный ему заказчиком код для последующей доработки. И возможно бюджет был всего лишь на оплату 0.5 человеко-часа.

Я сам сторонник того, чтобы писать идеальный образцовый well documented код без избыточности, дублирования логики и ламерства. Но если приходит заказ встроить например некий модуль в OsCommerce? Не стану же я переделывать более 1000 PHP скриптов, написанных много лет назад десятками разработчиков OsCommerce. Или если поручат дописывать функционал под CnCat... В общем, код спеца можно оценивать только если это именно им написанный с нуля код, и было выделено достаточно времени на разработку.


Совершенно согласен. Но вопрос был про качество. Качество и оценили. Были бы уточняющие моменты, тогда и оценивали бы по другому.
 А.
44 сообщения
#17 лет назад
Что вы имеете ввиду под уточняющими моментами?
Александр Х.
76 сообщений
#17 лет назад
alikfeld, озвучье стоимость, что тут секретного??? и предоставьте ТЗ! (со сроками изготовления), укажите посредников, если они есть, может вы заплатили посреднику n-ю сумму денег, а он заплатил гроши исполнителю.
А так все вилами по воде, нет ни одного конкретного примера, довода, просто вам плохо сделали и все! не ясны причины такой работы, может она кроется в заказчике?
Тут почти все заказчики выбирают тех, кто меньше возмет, быстрее сделает и не задумываются о последствиях, о том, что потом делать с тем или ным проектом.
Потом приводят пример плохого кода, и что теперь?
Александр Х.
76 сообщений
#17 лет назад
Нет ни одной ссылки, ни ТЗ, ни адреса исполнителя, ни отзыва, ничего нет, только ваши слезы и все, переходите на конкретику, может этот исполнитель, который вам сделал прояснит ситуацию?
Станислав Малкин
1410 сообщений
#17 лет назад
Цитата ("tvv"):
Насчет кода. Да, код ужасный. Мешанина HTML и PHP, логики и представления, без четкой структуры и с явными уязвимостями. Но не факт что разработчик совсем уж виноват. Возможно он использовал предоставленный ему заказчиком код для последующей доработки. И возможно бюджет был всего лишь на оплату 0.5 человеко-часа.

Я сам сторонник того, чтобы писать идеальный образцовый well documented код без избыточности, дублирования логики и ламерства. Но если приходит заказ встроить например некий модуль в OsCommerce? Не стану же я переделывать более 1000 PHP скриптов, написанных много лет назад десятками разработчиков OsCommerce. Или если поручат дописывать функционал под CnCat... В общем, код спеца можно оценивать только если это именно им написанный с нуля код, и было выделено достаточно времени на разработку.


Работа над этим проектом, со слов заказчика велась около полугода и все было писано с нуля. Откуда можно сделать вывод прямой о качестве работы.
Станислав Малкин
1410 сообщений
#17 лет назад
Цитата ("sxq"):
Прежде чем ругать исполнителя и автора этих скриптов, пусть заказчик опубликует ТЗ, по которому было все сделано и сумму, которую он оплатил, не спорю, что он мог заплатить и 500$, при этом ТЗ явно не соответсвовало этих денег, например, за 500$ хотел воспроизвести работу Яндекса
ArtLab, вы заказчик? вы так уверено говорите!


Нет, мы не заказчики. Нашей команде было поручено "подмести" за программистом, что это все наваял. После оценки кода - вердикт был неутешительным - работа сделана паскудно и с многими уязвимостями. Заказчик хотел убидится в этом не только с наших слов - мы предложили запостить на форум, чтобы люди тоже посмотрели и "оценили" . Такий файлов, как приведенный тут - около 100-150 штук. Допустим в одном файле по 1-2 уязвимости. Тогда в 150 файлах - их 300, а это просто кошмар.
Станислав Малкин
1410 сообщений
#17 лет назад
Цитата ("ADI"):
Ув. ArtLab, если позволите... Я хотел бы именно от Вас услышать недостатки данного кода..
Я не спорю - что их нет но все же....

Был бы очень благодарен. Интересно равнятся на професионалов...

ПС: просто все говорят - "плохой", "нормальный" но вот почему я услышал только от некотрых....

С ув. Андрей



Ув. Андрей !

Мы на звание профессионалов еще не заслужили, извините, но все-равно приятно

По делу:

Недостатки кода:

1) Потенциальные уязвимости, которые возможны при использовании register_globals=on.
2) Отсутствие разделения логики от представления , что делает чрезвычайно усложненным усовершенствование кода и его модификацию.
3) Отсутствие культуры написания кода ( отступы, форматирование )
4) Отсутствие знаний об SQL INJECTION. Вот пример кода:

Файл: showlot.php (к примеру страница

<?php

if(!isset($section)) $section=0;
if(!isset($sort)) $sort = 'domain_asc';
if(!isset($site_filter)) $site_filter=0;
if(!isset($domain_filter)) $domain_filter=0;
if(!isset($hosting_filter)) $hosting_filter=0;

session_start();

include('openmarketbase.php');
include('marketfunctions.php');
include('functions.php');

if(isset($login) && isset($password))
{
if($user=MarketValidUser($login, $password))
{
session_register('user');
}
else
{
if(session_is_registered('user'))
session_unregister('user');
}
}

if(session_is_registered('user'))
{
if(isset($lot_add) && $lot_add=='add')
{
include('phpmailer/class.phpmailer.php');

$price_passed = 0;

$offer_price = trim($offer_price);
if($offer_price!="") $price_passed=1;
$offer_price = ReplaceLtGt($offer_price);
$offer_price = ReplaceQuotes($offer_price);

$offer_text = trim($offer_text);
$offer_text = ReplaceLtGt($offer_text);
$offer_text = ReplaceQuotes($offer_text);

if(isset($safe_needed)) $safe_needed = 1;
else $safe_needed = 0;
if($price_passed) AddOffer($lot_id, $offer_price, $offer_text, $safe_needed, $user);
}
}



if(!isset($lot_id)) $lot_id = 0;
$lot = GetLot($lot_id);


Функция GetLot определена в файле marketfunction.php. Открываем его - находим нужную фунцию, смотрим:


function GetLot($lot_id)
{
$query = "select * from lots where lot_id=$lot_id";
$result = mysql_query($query) or die('Query failed from GetLot');
$lot = mysql_fetch_array($result, MYSQL_ASSOC);
return $lot;
}


Как видим - никакой защиты от SQL INJECTION нету, даже намека на это, что мол получили через адресную строку -то мол и суем. Я не буду объяснять, что это за уязвимость и что она за собой влечет - думаю каждый, кто хочет может прочитать про это тут: . В двух словах подобная пренебрежительность программиста влечет за собой возможность для взломщика "поиметь" сайт и всю его базу данных - а следовательно и в данном сайте - поиметь деньги всех пользователей.

5. Отсутствие проверок данных, что пришли от пользователя. Пример:


$offer_text = trim($offer_text);
$offer_text = ReplaceLtGt($offer_text);
$offer_text = ReplaceQuotes($offer_text);


И это все, что было проверено! А если я допустим захочу в переменную $offer_text к примеру вписать <div style="font: normal 72px Tahoma">Site Hacked By Vasya!</div> или что-то подобное - то этот скрипт просто положит это все в базу, а потом точно так же выведет это на сайт (при выборке из базы - проверок тоже нету) - и на сайте огромным шрифтом отобразится такая информация. Что подумает не искушенный пользователь зайдя на сайт? Правильно, подумает, что сайт действительно взломали. Доверит ли он свои драгоценные деньги такому сайту? Конечно, нет..

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


Если этих недостатков не хватает - могу еще накопать..там их пачки..
Андрей Л.
457 сообщений
#17 лет назад
ArtLab, но мы ведь говорили о другом "куске" кода?
Если можно самые "уязвимые" места того кода....если можно хотел бы увидеть как профи перепишут тот код....

С ув. Андрей
Станислав Малкин
1410 сообщений
#17 лет назад
Цитата ("ADI"):
ArtLab, но мы ведь говорили о другом "куске" кода?


Да, это все относится и к тому куску кода тоже. Чего-то и нету, но в целом этот весь код писал один и тот же человек. Какая разница какой кусок кода оценивать?

Цитата ("ADI"):

Если можно самые "уязвимые" места того кода....


Они уже были описаны выше.

Цитата ("ADI"):

если можно хотел бы увидеть как профи перепишут тот код....
С ув. Андрей


Что мешает? Обратитесь к профессионалам - они его перепишут для Вас. Сомневаюсь только, что бесплатно будут это делать.
Александр Г.
3 сообщения
#17 лет назад
Тянет на 10$ за скрипт
Андрей Л.
457 сообщений
#17 лет назад
Цитата:
1) Потенциальные уязвимости, которые возможны при использовании register_globals=on.

вот я например даже с on. Провожу инициааллизацию переменной...
Но эта "проблема"(если можно так назвать) очень распространима. Очень часто давали на доработку скрипты(причем не по улутшению безопасности, а просто абгрейд скрипта) и эти скрипты можно было легко обмануть по скольку нне знаем "от куда пришла переменная".
_____
Ну а на счет SQL запросов... - полез в старый архив(первые скрипты мои) и в админке гостевой нашел код с "закрыытими кавычками
Хотя когда начинал точно повню что не знал о Register_Globals (то есть последсвия). Просто взял стандартый код и так писал... писал.. ну конечно же усовершенсвовал свои уммения с каждой строчкой.....
________
если чесно то я сам иногда делаю код - "грязным" - это по мнению других..И делаю намеренно по скольку я с ним быстрее работаю...
Конечно же другому програмисту будет тяжело(((
В фаайлах которые разрабатываю коментирую все порой даже комета больше чем кода....
Андрей Л.
457 сообщений
#17 лет назад
ArtLab, Спасибо! Думаю тема исчерпала себя!?
 Александр
4 сообщения
#17 лет назад
Ну да, то о чём я говорил. Все дырки меркнут против SQL-запросов.
Эдуард К.
45 сообщений
#17 лет назад
2tvv:
Цитата:
И возможно бюджет был всего лишь на оплату 0.5 человеко-часа.


Вспомнилось на счет беременности наполовину Если я могу выполнить задание хорошо, то я либо выполню хорошо, либо не возьмусь вообще, если денех недостаточно

2sxg:
Цитата:
озвучье стоимость, что тут секретного???

А нахуано? *чешет репу* Что это нам прояснит?

Цитата:
только ваши слезы и все

Какие слезы? Человек поинтересовался, на сколько качественен код - вот и все. Решение ему принять надоть, никого ж не обвиняет ни в чем





2Spaceman:
Цитата:
Все дырки меркнут против SQL-запросов.

Фигня. Эт только речь об уязвимостях. А када дело до приличных нагрузок дойдет, то и в неуязвимом коде проблем будет выше крыши
Дмитрий Вашкевич
35 сообщений
#17 лет назад
Цитата:
Фигня. Эт только речь об уязвимостях. А када дело до приличных нагрузок дойдет, то и в неуязвимом коде проблем будет выше крыши

Вы упали? Хотите сказать, что использование закавычивания создаст существенную нагрузку на сервер?
Использование include ('template/anybody.php'; будет тормозить по сравнению с текущим вариантом?

Тогда можно смело сказать, что код тормозит изначально, так как, например, используется echo "...". Двойные кавычки. Содержимое обрабатывается дольше, чем в одинарных.

Цитата:
.........
echo "<form action='' method='get' class='lightgraytext' style='text-align:center;'><table style='width:514px; margin-top:20px; margin-bottom:20px; text-align:center' align=center>
<tr><td>Показать:<td><a href='?section=$section&page=$page
............



Гыыыы.....
Эдуард К.
45 сообщений
#17 лет назад
Churchik, Цитата:
Хотите сказать, что использование закавычивания создаст существенную нагрузку на сервер?

Нет, я вообще о структуре БД (в частности о выборе правильных типов индексов в зависимости от превалирующих операций, чтение или запись) и оптимизации запросов на работу нескольких сот пользователей одновременно
Сергей В.
244 сообщения
#17 лет назад
Есть во первых хороший тон, такой как оформление кода.
Во вторых есть хорошее правило, код php отдельно, html отдельно, в разных файлах. В html коде миниум php вставок.
В третьих это комментарии
Ну и в четвертых как посоветовали выше это есть куча шаблонизаторов, например Smarty (smarty.php.net).
Поверьте облечает работу и логику. Так же скорость работы скрипта тоже гораздо быстрее.
Ad R.
62 сообщения
#17 лет назад
Использование глобальных переменных вообще нонсенс, это уже когда совсем соображалка не работате.