Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I've got a legacy PHP web application wherein almost each and every function makes references to $_POST variables - retrieving their values, AND setting them (or setting new POST variables) as a means to communicate with the calling code or other functions.

eg:

function addevent()
{
...
$add_minutes=$_POST['minutes'];     
...
$_POST['event_price']=''; 
...
}

I would have thought that the straightforward approach would have been to pass to a function all the values that it needs, and return all what they generate.

As an old-school programmer, albeit a bit out of touch now, I find this structure grotesquely unsettling - where data is passed arbitrarily all over the place. What do others think? Is the above approach acceptable now?


Edit 1

One conceivably genuine use-case is as follows. This is a function which handles embed tags or an uploaded file.

function brandEvent($edit_id='')
{
...
    switch($_POST['upload_or_embed'])
    {
        case 'embed':
        ...
        // uses $_POST['embed_video_code']
        ...
        break;
        case 'upload':
        ...
        // uses DIFFERENT POST variables
        ...
    }
}

Is this a sensible code structure?

share|improve this question
    
codereview.se seems more appropriate for this question. –  Chris Jul 30 '11 at 12:33
1  
@Chris: I disagree, because he's not trying to get his own code fixed up. –  DeadMG Jul 30 '11 at 12:49
1  
@DeadMG: Perhaps this does fall under "Development Methodologies" or "Design Patterns" from Faq. –  Chris Jul 30 '11 at 13:41
add comment

2 Answers

up vote 4 down vote accepted

As an old-school programmer, albeit a bit out of touch now, I find this structure grotesquely unsettling - where data is passed arbitrarily all over the place. What do others think?

I, too, find this structure grotesquely unsettling. Those functions should be atomic, independent of external influences. Your examples can be called programming by coincidence and this is a horrible practice. It's unrobust code depending on side-effects and globals and will likely cause headaches for maintenance programmers. This style also effectively limits the reusability and testability of those functions, as they are so tightly coupled to a specific POST of variables (probably from the view).

This approach is by no means acceptable.

share|improve this answer
    
Thanks - any thoughts on reasons besides maintainability? Could you point out some programming standards / approaches it violates? @DeadMG ? –  matt74tm Jul 30 '11 at 23:56
    
I pretty much pointed out all there is to say. This style is unacceptable for the reasons I mentioned. And those are some very good reasons. I know it's hard to communicate them to inexperienced developers. Bad reusability, testability, maintainability, dependence on globals...that's probably the worst you can say about code at all. –  Falcon Jul 31 '11 at 10:08
1  
@matt74tm: Just imagine you wanted stateful URLs with this code ... –  Falcon Jul 31 '11 at 20:57
add comment

As an old-school programmer, albeit a bit out of touch now, I find this structure grotesquely unsettling - where data is passed arbitrarily all over the place. What do others think? Is the above approach acceptable now?

No, the above approach is completely not acceptable now. If anything, it used to be a lot more acceptable than it is now. Such code is virtually impossible to reason about and would be an incredibly unmaintainable mess. You're absolutely right to label it as hideous.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.