example of update

[fiedsch]

as proposed in https://github.com/contao/docs/issues/324 this is a first change. Please also see the following comment.

[fiedsch]

I’d like your ideas on the following:

• Should we delete the „It passes … and does not expect a return value“ in the description as the parameters are described in the following section anyways?
• How about the return value? You should be able to see this from the example code but an explicit mention would be better. Should this be a new section right after ## Parameters?
• I deleted the PHPDoc comments in the example as it does not make sense to me to repeat what just has been said in ## Parameters. On the other hand---if the example is used for copy/paste---it would be nice to have the parameter descriptions there.

[aschempp]

It depends on what we want the intro to be. It can either be a "when" or it could be a summary. I'm fine with both, but I think we should not write about the return value like in your example. If we remove the parameter, then let's remove the return too.

Regarding the PHPDoc, I agree that's unnecessary in an example. Every good IDE will generate that on the fly, which is way more helpful.

Also, I suggest to change $registration to $registrationModule or just $module. I usually name my variables after their type, and it's not a registration.

[fiedsch]

Thanks for your comments. I made some small changes to the files:

• Added a section "Return value" right below "Parameters. I would prefer to always have this, even---as in this case---when there is no return value. It might also help in cases where there is no return value but parameters are passed by reference so they can be modified. This could be mentioned there as well.
• renamed the parameters to $registrationModule as you proposed and changed $member to $memberModel to be consistent and because $member does not indicate if it is an instance of FrontendUser or MemberModel (http://www.commitstrip.com/en/2015/10/27/one-of-the-coders-hardest-problems/).

Oppinions on that?

[lionel-m]

Added a section "Return value" right below "Parameters. I would prefer to always have this, even---as in this case---when there is no return value. It might also help in cases where there is no return value but parameters are passed by reference so they can be modified. This could be mentioned there as well.

:+1:

[aschempp]

Valid point with the renaming.

Regarding the return values: This is similar to PHP. We are not adding @return void because the default is not to return something. Actually it was always wrong to return stuff in hooks, but that's just the way they are built. Also regarding references, I don't know why we would need to explain that every time, because that should be basic knowledge of a PHP developer? ;-)

[fiedsch]

Also regarding references, I don't know why we would need to explain that every time,

I think you got me wrong. I don't mean to explain call by value vs. call by reference. But if (an estimated) 90% of the hooks return a value and just some expect that you modify a parameter that is passed by reference I think that is worth a mention.

[aschempp]

Well, I think that none expects you to modify a parameter. The ones that expect something (like replaceInsertTags) have a return value. The others (like prepareFormData) expect you to do something (like store the data) and not modify anything.

[fiedsch]

Well, I think that none expects you to modify a parameter.

Not right if you look at (e.g.) myGetCountries()

[aschempp]

I agree that's a kind of strange hook, which should have a return parameter. Here it would make sense to describe the reference, but to me it does not for prepareFormData.

[fiedsch]

I agree that's a kind of strange hook, which should have a return parameter. I'm with you on that. But we have to describe what is (instead of what should be ;-)

Here it would make sense to describe the reference

I think that is already in the description

but to me it does not for prepareFormData.

To be honest, I don't understand why. For me it's the same situation and hence should be described the same way.

[aschempp]

It's not, because the hook is not meant to override/modify the form data, its meant to process it.

[fiedsch]

I meant "same" as being technically the same. Not the intention of the method. But the original PR was just a proposal and a request for comments by others. I don't really mind which way it will be finally documented. I'd only like it to be consistent.

Thanks for your input.

PS: Just saw I confused the hooks.

[fiedsch]

It has been a while when I last worked on this. Since I'm not yet friends with git :-( I'd like to start from a fresh fork so I don't mess anything.

To sum up:

• remove the "It passes ..." in the intro as the description of the parameters immediately follows
• do not introduce a ## Return value section
• keep the example code as in this PR

Does that make sense?

Thanks for your ideas.

[aschempp]

That sounds right for me :-) I would suggest to create a new pull request based on the latest master branch. If you agree, please close this :-)

[fiedsch]

I will prepare a new (fresh) PR