Support the ongoing development of Laravel.io →
posted 8 years ago
Architecture
Last updated 2 years ago.
0

I am still pretty new to Laravel, so I am not yet familiar with the best practices when using it, but I do have a lot of experience with enterprise Java so I can give you some generic advice.

The first thing I would do is break up your process into discrete high-level "steps". For instance, these are some of the steps I see:

  1. retrieve uploaded file
  2. create $campaign object
  3. unzip uploaded file
  4. iterate over files that were uzipped

and so on

So I would start by making functions for each of those steps. For now, just put those functions as methods on your controller. And for each function, you should think in terms of what you should pass in, and what you want it to return.

So for instance, with your retrieveUploadFile() function, you have some choices. For inputs, you could pass in either $request or $request->file('file'). Passing in $request->file('file') would be my preference since it makes that function more re-usable, in the case that you use it somewhere else and you call your file upload param something other than 'file'.

For outputs, you can either have it return true or false, to indicate success or failure, or you could have it return the path of the uploaded file on success, and false or null on failure. My preference would be the latter, because it's more explicit. Also, by not returning the path, you're requiring all the code that uses this function to know implementation details of this method, in particular that it uses getUploadPath() as the location where it stores files. And then if you ever wanted to change the implementation so that it used a different location, you'd have to update every place that called that function to use the new location.

So once you have this logic extracted out into it's own method, you need to determine how re-usable it should be. If it's only going to be used in this one controller, just keep it on the controller. If you know it's going to be used in other places, you need to put in somewhere that it can be accessed. In Laravel, I think this means creating a service provider, but I am not 100% sure on that. Still a little fuzzy there. If you aren't sure if you are going to use this method in other places or not, you should keep it on the controller, and then if and when you decide you need it in other places, then move it to a service factory at that point. I would caution against extracting it out to a service before you are sure you need that.

So just a few other thoughts. When you start making functions from the discrete steps you identify, the function names should describe the step so that when you read through your main method it reads almost like a story. "First, I $this->retrieveUploadFile(). Next, I $this->createCompaign(). After that, I ..."

Also, I said that you should extract the methods first, and then decide how much to expose them second. But really, those happen at the same time. If you know that a function is only going to be called in this one place, you may make it a little more specific to what you are trying to do than if you were making one that you were going to be calling it from multiple places in your code.

Lastly, specific to the file upload code, you have a short-circuit condition where if the file was not included in the request you just return an error. The short-circuit condition is not part of the file upload code, it's the result of what happens when the file upload code fails, so you would keep that logic in your main method:

$fileUploadPath = $this->retrieveUploadFile($request->file('file'));
if (null === $fileUploadPath) {
    return back()->with('error', 'no file uploaded');
}

I hope that helps.

Last updated 8 years ago.
0

Thank you :) i wasnt expecting a detailed reply as that.. so thanks for going into detail :)

0

Sign in to participate in this thread!

Eventy

Your banner here too?

shez1983 shez1983 Joined 31 Dec 2014

Moderators

We'd like to thank these amazing companies for supporting us

Your logo here?

Laravel.io

The Laravel portal for problem solving, knowledge sharing and community building.

© 2024 Laravel.io - All rights reserved.