1
votes

I'm working on a project where the users have to upload several images (through single form field) to a server. Using foreach, I'm storing the images using ftp. Here's my code:

use Storage;
$image_list = $request->file('images');
foreach($image_list as $image){
  $filename = Cuid::slug().'.'.$image->getClientOriginalExtension();
  $path = "images/templates/";
  Storage::disk('ftp')->put($path.$filename, 'Contents');
}

This code actually works fine but I'm afraid using storage facade directly like this can create several instances for every loop and It's not optimised code anymore. I tried creating an object for storage facade like this:

$storage = new Storage;
foreach($image_list as $image){
  ...
  $storage->disk('ftp')->put($path.$filename, 'Contents');
}

But this gives me an error saying:

Call to undefined method Illuminate\Support\Facades\Storage::disk()

Is there any way not to call Storage facade directly every time the loop runs?

1

1 Answers

2
votes

This code actually works fine but I'm afraid using storage facade directly like this can create several instances for every loop and It's not optimised code anymore.

As per my understanding, you are worried that the code that you have made is not optimized because you use the facades directly.

The Facades are not necessarily creating new objects every time you use a facade, rather it resolves out of the container (think of it as a blackbox and it will give you what you asks for), sometimes creating a new object, but most often it still give you the already existing object.

In case you are wondering how this works: google for singletons and memoization.

So, in this case, you can use the same code without worrying. and the code you are trying to propose is still the same. But I'm sure that you will hit performance issues, But not now.

One thing I can think of now is to modify your code in a way that you can try saving/putting many files at once rather than sending one file at a time via FTP? or maybe use a queue?.