Overriding methods with different parameters

@martinbean ah! That would explain it then :) Thanks!

So can I ask you how you would solve this instance? Is mine just a poor design, or should I maybe manage the form request inside the method? Or something better?

@bwrigley I sorely wish PHP supported method overloading for a use case spookily similar to yours.

I primarily use resource controllers in my applications, and I’d love to be able to define an abstract ResourceController and then have various child controllers extend it with more specific types. Unfortunately, I’ve not really found a “nice” way to achieve this.

@martinbean I want to use resource controllers too as they are straightforward and make sense to me. This hiccup leaves me with three options I can think of, but none of which I like!

1. Remove the overloading altogether and just duplicate code for each model.
2. instead of overloading methods, have methods with different names in children e.g. _store()
3. In this particular case, instantiate the form request inside the method, but how long before I find myself with another method that needs different parameters?

    public function store(): RedirectResponse
    {
        $request = app('App\Http\Requests\AdvertStoreRequest');

        $validated = $request->validated();
		 //
   }

Thanks @kokoshneta! I've tried the former but I get the same error unfortunately :(

@bwrigley Have you made sure that the actual signatures are also compatible? If you make a generic StoreRequest class and extend that in your PostStoreRequest and AdvertStoreRequest, then your store method should typehint for a StoreRequest – the parent class, not the child classes. You can then pass either of the child classes as the actual parameter, and they will be accepted since they both extend StoreRequest.

Although, as far as your snippet shows, the method does the same thing in both controllers, so there’s little need to define it more than once.

I would define the PostController like this:

class PostController extends Controller {
	protected $postableType = PostType::GENERIC;
	protected $modelType = Post::class;

	public function store(StoreRequest $request): RedirectResponse {
		$validated = $request->validated();
		$post = new $this->modelType;
		$post->message = $validated['message'];
		$post->user_id = $request->user()->id;
		$post->postable_type = $this->postableType;
		$post->save();
		return Redirect::route('feed');
	}
}

And the AdvertController would just need this:

class AdvertController extends PostController {
	protected $postableType = PostType::ADVERT;
	protected $modelType = Advert::class;
}

@bwrigley if AdvertStoreRequest extends PostStoreRequest and you are still getting an error you have left out some important information

@kokoshneta Thanks again for taking the time on this.

I'm not sure I've fully understood how this works.

You are right that my two store methods are the same, the only reason I have overridden one is because I want to use a different FormRequest for each store method as the validation rules are slightly different between the Post and the Advert models.

In your example I can see that PostController::store() will know to use the PostStoreRequest form request to validate input, but how will AdvertController::store() know to use the AdvertStoreRequest form request?

Sorry if I'm being dumb:

My PostStoreRequest:

class PostStoreRequest extends FormRequest
{

//

    public function rules(): array
    {
        return [
            'message' => 'required',
            'parent' => 'null'
        ];
    }

//

}

My AdvertFormRequest:

class AdvertStoreRequest extends FormRequest
{

//

    public function rules(): array
    {
        return [
            'message' => 'required|max:255',
            'parent' => 'null'
        ];
    }

//

}

@krisi_gjika hmm I'm not sure what that could be? Important information in the FormRequests you mean? Or in the Controllers?

@krisi_gjika They haven’t “left something out”. Method signatures—including parameters—need to match exactly when extending classes. PHP has always behaved like this.

@bwrigley Oops, sorry – I had neglected to edit a crucial part. The signature in the store() method should call for a StoreRequest, not a PostStoreRequest! I’ve fixed that in the comment above now.

This is assuming that PostStoreRequest and AdvertStoreRequest both extend StoreRequest. Those two child classes will then not need to define the store() method at all, because type-hinting for StoreRequest automatically, implicitly also allows any classes that extend StoreRequest.

So when you store a generic post in your PostController, you pass a PostStoreRequest object as the $request parameter; when you store an advert in your AdvertController, you pass an AdvertStoreRequest object.

Although, come to think of it, if you’re relying on the service container to dependency-inject the request, then it will need to know the final class, not the parent class. You could get around this by instantiating the request inside the method (as you mentioned above), based on information available in the controller; for example:

class PostController extends Controller {
	protected $postableType = PostType::GENERIC;
	protected $modelName = 'Post';

	protected function getModelInstance() {
		$class = "\\App\\Models\\" . $this->modelName;
		return new $class;
	}

	protected function getRequestInstance($type) {
		$class = "\\App\\Http\\Requests\\" . $this->modelName . $type . "Request";
		return new $class;
	}

	public function store(): RedirectResponse {
		$request = $this->getRequestInstance('Store');
		$validated = $request->validated();
		$post = new $this->getModelInstance();
		$post->message = $validated['message'];
		$post->user_id = $request->user()->id;
		$post->postable_type = $this->postableType;
		$post->save();
		return Redirect::route('feed');
	}
}

class AdvertController extends PostController {
	protected $postableType = PostType::ADVERT;
	protected $modelName = 'Advert';
}

@kokoshneta Ah yes thanks. That's similar to where I had got to with my options to @martinbean above. I think it's probably the way I'll have to go. Thanks again!

@s4muel Really fair point :)