I am working on a project, the previous programmer used this method to save photos, videos, etc.

 if (empty($data['large_image'])) {
$large_image = $article->large_image;} else {
if ($request->hasFile('large_image')) {
    $cover_img_large = $data['cover_img_large'];
    //echo '<pre>'; print_r($cover_path);die;
    if (file_exists($cover_img_large)) {
    $image_path = $data['large_image'];
    $extension = $image_path->getClientOriginalExtension();
    $New_path = rand(111111, 999999999) . '.' . $extension;
    $path_path = 'upload/articles' . '/' . 'large' . '/';
    $large_image = $path_path . $New_path;
    if (!File::isDirectory($path_path)) {
        File::makeDirectory($path_path, $mode = 0777, true, true);
    Image::make($image_path)->resize(800, 460)->save($large_image);

I intend to use this method, which is close to the structure of the strategy design pattern.

interface Image{
public function image($model,$nameInput,$path);}

class saveArticle implements Image{

private $local_path="upload/articles/";

public function image($model,$nameInput,$path){
    $request = new Request();
    $data = $request->all();
    $path = $this->local_path.$path.'/';

    if (empty($data[$nameInput])) {
        return $model->large_image;
    } else {
        if ($request->hasFile($nameInput)) {
            $image_path = $data[$nameInput];
            $extension = $image_path->getClientOriginalExtension();
            $New_path = rand(111111, 999999999) . '.' . $extension;
            $name_image = $path . $New_path;
            if (!File::isDirectory($path)) {
                File::makeDirectory($path, $mode = 0777, true, true);
            Image::make($image_path)->resize(800, 460)->save($name_image);
            return $name_image;


When I use this method, I have a very simple time to use it and like the following code, I can store information with only one line.

$article = Article::find(1);
$save = new saveArticle();

I have an easier task to develop the code and I can use the same method to add a movie or podcast and just add a new interface. What is your opinion? Do you think this is the right way? Thank you for guiding me. Your comments are valuable to me.

  • \$\begingroup\$ Welcome to Code Review! Is the first snippet part of a controller method? could you please edit to include more code - e.g. it might help reviewers to know where $request, $data come from (even if it is method parameters). \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Apr 30 at 14:35
  • \$\begingroup\$ First and foremost, you never check the MIME type of the file allowing any filetype to be uploaded. Furthermore, You do not show what File::makeDirectory() does. I hope there is no sys calls because you never escape anything that comes from the data to protect from any arbitrary command injections. \$\endgroup\$ – Jaquarh May 1 at 10:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.