CodeIgniter Forums

Full Version: CONTROLLERS, single method or different methods for different actions?
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Pages: 1 2
Let's say I have a Controller and a View where you can edit a customer (where 123 is the customer's unique ID):

/customer/edit/123

The view might be made up of some regular form fields, for example Name, Email Address, Telephone Number, etc.  The database table in the related model might be called "customers".

But say the View also features a "sub table" - for example a list of the customer's favourite colours.  I want to be able to add colours to the list, delete colours from the list and change the order of preference of the colours.  I'd also want to be able to edit the list of colours via Ajax.  There would be a separate database table in the model perhaps called "customer_colours"

This leaves me with one of two options:

1) In the view I could have a single form which posts to /customer/edit/123 and then do some jiggery poker to figure out whether the user is attempting to edit the core customer fields, or whether they're adding/editing/deleting a colour, and whether the request is a regular one or an Ajax call.

I tried this and I ended up with a humongous Controller method.  I tried breaking it down into a master controller method which then calls private sub methods depending on the required action, which was better but still somewhat of a mess.

2) The other option is to create multiple form tags in the View, such as /customer/edit, /customer/colour_delete, /customer/colour_add, etc which then correspond to the appropriate methods in the Controller, but all of which output the same View.  This feels a lot more organised, but there are then limitations as to how the forms in the View can be laid out.

Is there a method which is more preferable over another?
You would want to add separate methods like add, edit, delete etc;
Quote:Let's say I have a Controller and a View where you can edit a customer (where 123 is the customer's unique ID):
/customer/edit/123

do not do this. i know its in almost every api tutorial to do it this way - but if you have any choice at all do not because exposing the customer id, or a db record id in the URL for a page that you need to be logged in to access -  is insecure.

Quote:then do some jiggery poker

sounds legit  Dodgy

so here is another way - yes (obviously) you should have separate methods, because you need to call different validation etc. but you also can call the same form url in your different forms - just put a hidden form field like
PHP Code:
form_hidden'task''editcolors' ); 

then in the controller method that the form goes to - pick up the 'task' and route to the correct method based on the hidden form field - and make the method private like _editcolors()
(05-10-2016, 02:24 PM)cartalot Wrote: [ -> ]
Quote:Let's say I have a Controller and a View where you can edit a customer (where 123 is the customer's unique ID):
/customer/edit/123

do not do this. i know its in almost every api tutorial to do it this way - but if you have any choice at all do not because exposing the customer id, or a db record id in the URL for a page that you need to be logged in to access -  is insecure.

No it's not, at least not in most cases.

It's a minor information leak at best, and could be classified as such solely because of the logical argument that, if something requires login to be seen, it is therefore a secret to unauthenticated users.
Following this line of logic, anything that is accidentaly seen by outsiders is an information leak - even silly stuff like visitors count for the past hour.

You need to perform risk assessment (a fancy term for "think about potential dangers") to really know if leaking a single customer ID could be harmful in any way, but chances are that (unless you require login for all users) you'd have totally public links exposing the same IDs anyway, making the above argument void.

Even if you decide that exposing these IDs is indeed a risk, API design is not the issue; it's the scheme used to generate the IDs in the first place.

Speaking of which, a potentially way more serious issue (and possibly the reason why you've heard/read the advice you're echoing here) is using such numeric, sequential IDs, especially when authentication is not required.
It doesn't matter if they appear in a URL, a POST form field, etc. It's the IDs deterministic nature that can bite you ... Imagine a dating site using the very popular AUTO_INCREMENT-ed integers for user ids, and without any further safe-guards, using the same IDs to name uploaded user pictures - a one-line command would be able to download ALL pictures on the website, resulting in a major privacy breach.

(05-10-2016, 02:24 PM)cartalot Wrote: [ -> ]and make the method private like _editcolors()

Nitpicky, but ... s/private/inaccessible/
Terminology is important. Smile

CodeIgniter may prevent controller methods prefixed with an underscore from being publicly accessible, but that's just a legacy feature left from the PHP4 era when the underscore-prefixing convention was used to denote methods that shouldn't be called from outside of that class.
Technically, it's just an informational thing; CI developers at the time have decided to make it functional due to lack of better alternatives.

But we're in the PHP7 era now and "private" has a specific meaning. Also, making controller methods really private by using the keyword works just fine - you don't need to prefix those with an underscore and they still won't be routable.
(05-10-2016, 11:55 AM)InsiteFX Wrote: [ -> ]You would want to add separate methods like add, edit, delete etc;

Yes I realise this, but the question is whether my view has a single form routing to a single method, and that method then determines what the user wants to do. Or whether I have a separate form for each action.

(05-10-2016, 02:24 PM)cartalot Wrote: [ -> ]sounds legit Dodgy

so here is another way - yes (obviously) you should have separate methods, because you need to call different validation etc. but you also can call the same form url in your different forms - just put a hidden form field like
PHP Code:
form_hidden'task''editcolors' ); 

then in the controller method that the form goes to - pick up the 'task' and route to the correct method based on the hidden form field - and make the method private like _editcolors()

Well that is kind of what I meant by Jiggery Poker. Another option would be to give different buttons name and value, so the controller knows if the user hit "save" or whether they hit "delete colours"

But if I had separate forms all going to the same controller, using a hidden task field, had I might as well not just have separate forms routing to separate controller methods?

(05-10-2016, 03:28 PM)Narf Wrote: [ -> ]Nitpicky, but ... s/private/inaccessible/
Terminology is important. Smile

CodeIgniter may prevent controller methods prefixed with an underscore from being publicly accessible, but that's just a legacy feature left from the PHP4 era when the underscore-prefixing convention was used to denote methods that shouldn't be called from outside of that class.
Technically, it's just an informational thing; CI developers at the time have decided to make it functional due to lack of better alternatives.

But we're in the PHP7 era now and "private" has a specific meaning. Also, making controller methods really private by using the keyword works just fine - you don't need to prefix those with an underscore and they still won't be routable.

Thanks for the clarification. I've been doing private methods as:

private function _my_private_method() { }

(05-10-2016, 03:28 PM)Narf Wrote: [ -> ]
(05-10-2016, 02:24 PM)cartalot Wrote: [ -> ]
Quote:Let's say I have a Controller and a View where you can edit a customer (where 123 is the customer's unique ID):
/customer/edit/123

do not do this. i know its in almost every api tutorial to do it this way - but if you have any choice at all do not because exposing the customer id, or a db record id in the URL for a page that you need to be logged in to access - is insecure.

No it's not, at least not in most cases.

It's a minor information leak at best, and could be classified as such solely because of the logical argument that, if something requires login to be seen, it is therefore a secret to unauthenticated users.
Following this line of logic, anything that is accidentaly seen by outsiders is an information leak - even silly stuff like visitors count for the past hour.

In my case this is a system which requires users to login, and those said users will all be related (i.e. part of the same organisation) so no data should be leaked or accessible to the outside. For me personally, it makes no odds whether the customer ID is in the URL or in a hidden field, since a right-click and "view source" is hardly a deterrent to anyone who knows what they're doing.
You can configure the form_validation to do both using an editFlag switch.

But it will make it more difficult to maintain in the method and view.
just to be clear - as soon as you are talking about design - its just peoples opinions. so with that said

Quote:But if I had separate forms all going to the same controller, using a hidden task field, had I might as well not just have separate forms routing to separate controller methods?

for me - the advantage is that i only have one public method in the controller and the only thing that method is doing is routing to the different private methods depending on which form it is. this keeps the forms simpler - they all go to the same place. and it keeps the controller safer because none of the form methods are public. and a small benefit is being able to look at the public method in the controller and very quickly see what forms are calling on it and which methods they route to.
Quote:For me personally, it makes no odds whether the customer ID is in the URL or in a hidden field, since a right-click and "view source" is hardly a deterrent to anyone who knows what they're doing.

i'm not talking about people doing view source, and i prefaced it by saying its not a popular opinion which narf expanded on. but narf also pointed out the extremely critical follow up which i would suggest you read carefully.
(05-10-2016, 03:28 PM)Narf Wrote: [ -> ]But we're in the PHP7 era now and "private" has a specific meaning. Also, making controller methods really private by using the keyword works just fine - you don't need to prefix those with an underscore and they still won't be routable.

(hangs head in shame) ok not really, but - yeah i thought the underscore prefix for the method name was still a 'convention' for helping to identify private methods in php - and it was including zend in early 2015 - but apparently in late 2015 it was removed in zend and the consensus is now not to use it.
Personally I think it is not a great idea to have sequential ids anywhere, as they are easy to guess or programatically run through, not in forms or urls or anywhere. Except of course in trivial matters, or perhaps where you are using say a page id, but even then, you should use a page url code and decode that to a page id so the url is nicer.

As for the colour removal, I would do the customer form as a normal form, but deleting and adding colours as a javascript function, so when the user deletes a colour it just pops off the screen (with a 'saving changes' pause in the middle - I always love using those animated loading gifs), without the user having to refresh the page for such a minor thing.

Alternatively, make the 'Edit Colours' a seperate form altogether. It is just a matter of user interface design. For the user sometimes doing this in a blocky way can be easier to follow. "How do I change my favourite colours?" "Ah, here is a big button that takes me to a nice clean screen that enable me to do that".

For me, for speed of coding I would do the blocky multi page approach, for quality experience I would do the javascript approach. Personally I try to avoid using 'flags' in hidden form fields as much as possible.

Hope that helps,

Paul.
Pages: 1 2