Покритикуйте код

[proWoke]

Вот кусок кода из админки. Он принимает из формы файлы с фотками и поочереди их называет и сохраняет в папку.

if (isset($_REQUEST['addphoto']))
{
// По выбранному разделу формируем путь к нужной папке
$pathtoserver = getenv("DOCUMENT_ROOT");
$clickdir = $_REQUEST['section'];
$pathtodir = $pathtoserver."/ushkov/blocks/$clickdir/bigimg/";
// Создаём массив выбранных фотографий
$data = array (
	$_FILES['photo1'],
	$_FILES['photo2'],
	$_FILES['photo3']
	);
// Поочерёдно загружаем каждое фото
	foreach ($data as $filename) 
	{
		$tmp = $filename['tmp_name'];
		if (file_exists ($tmp)) 
		{
			$info = getimagesize($filename['tmp_name']);
			if (preg_match ("{image\/(.*)}is",$info['mime'] ,$pocket))
			{
			$name = $pathtodir.mt_rand(1,100000).".".$pocket[1];
			move_uploaded_file ($tmp, $name);
     			header ("Location: index.php");
			}
		
		} else { header ("Location: index.php"); }
	}
}

Покритикуйте, пожалуйста. Если он достоин этого.

 

[флоппик]

1. Никак не уведомляешь пользователя о успешности/ошибочности/причине ошибки.
2. Почему ограниченно тремя фото? Почитай про отправку массивов из форм.
3. Зачем код загрузки файлов жестко завязан на специфичные для проекта папки? их надо отделить, что бы код можно было безболезненно переносить между проектами.
4. Почему имя файла формируется случайным числом, которое обладает слабой выраженной уникальностью?
5. При больших количествах фото в одной директории ФС будет тормозить - разумно сделать разделение по подпапкам.
6. getimagesize выбрасывает warning, если файл не является картинкой. Бред, на самом деле, но я бы предпочел задавить собакой.

 

[Духовность™]

Критиковать-то в общем нечего)

У меня для загрузки файлов написан загрузчик:
абстрактный класс - http://pastebin.com/RqxsSarW
конкретный - http://pastebin.com/6Af08Abr
работает как-то так вроде: http://pastebin.com/Zh0U5wAv

правда я ими ни разу ещё не пользовался

 

[Breeze]

$clickdir = $_REQUEST['section'];
$pathtodir = $pathtoserver."/ushkov/blocks/$clickdir/bigimg/";

А мне вот это не нравится. Потенциально вредно.

if (file_exists ($tmp))

для этого есть is_uploaded_file

 

[fixxxer]

triumvirat

99% комментариев можно безболезненно убрать, если назвать методы и переменные так, чтобы они не врали о своем назначении.

например:
- public function copy($directory)
+ public function copyTo($directory)

- protected $file_directory
+ protected $upload_directory


checkOtherErrors - лол.

 

[Духовность™]

ну да, согласен. сейчас посмотрел на код и массу подобных исправлений нашел. эх, опять рефакторить все ((

 

[Ragazzo]

кстати раз топик такой, может кто-нибудь подскажет как сделать многочисленную загрузку файлов на серв (вариант с кучей инпутов типа "файл" катит), мне что-то вроде модального окна, где появляеться диалог выбора файла, и потом когда выбраны файлы, несколько они оправляются на серв....для тех кто вконтакте есть, что-то вроде отправки фото...а то десять штук например "<input type="file">" некрасиво очень...

 

[Духовность™]

http://www.swfupload.org/

сейчас с ней работаю. сука, немного глючная, но мне нравится. конкретно по твоему вопросу наверно это: http://demo.swfupload.org/v220/applicationdemo/index.php

 

[Ragazzo]

triumvirat
Спасибо, щас демо глянул, хорошая такая....вот кстати нашел на jquery что-то...щас особо не вникал
http://habrahabr.ru/blogs/jquery/50223/

 

[Adelf]

Я недавно делал такую загрузку.. и по какой-то причине меня больше устроил http://www.uploadify.com/ чем swfupload. Причину уже не помню

 

[Духовность™]

вспомни.

офф: как тут по никам обращаться к собеседнику?

 

[Ragazzo]

я только BB тегами понял как)

 

[fixxxer]

вспомни.

офф: как тут по никам обращаться к собеседнику?

Copy paste только, увы. Кто-то там в соответствующем треде уже постил js для этого, но у Саши походу времени нет этим заниматься