[Coding style] Use hooks

Topic: [Coding style] Use hooks (Read 5019 times) previous topic - next topic

0 Members and 1 Guest are viewing this topic.

[Coding style] Use hooks

April 04, 2014, 03:08:14 am

I was wondering: what do you think if while coding Elk we start using the hooks to provide "core" functionalities too?
For example (just because I'm looking to the post page), load the "Load drafts" interface, not "hard-coding" it, but using the hooks available in the system already.

Why that proposal?
Well, the advantages I see are: 1) we can test "live" the hooks, 2) we can easily see when and where hooks may be useful (if we can't move a feature to the hooking system something may be wrong), 3) it would help decoupling the code making it hopefully easier to maintain and expand, 4) it would allow easier switching on/off of features (I just made that one up now

).

Opinions?

Re: [Coding style] Use hooks

Reply #1 – April 04, 2014, 03:16:33 am

Sounds like a nightmare.

Re: [Coding style] Use hooks

Reply #2 – April 04, 2014, 07:53:52 am

emanuele is tempted to warn Antechinus.

Re: [Coding style] Use hooks

Reply #3 – April 04, 2014, 08:31:31 am

You asked for opinions. If you ask for opinions, you're likely to get them.

Re: [Coding style] Use hooks

Reply #4 – April 04, 2014, 12:05:16 pm

Agree it sounds painful. But post does need refactoring. I've mentioned before what a pain it was to alter permissions as it's checked in so many places in so many different ways.

Breaking it into consistent functions would be nice, but I'm not sure those need to be called through hooks. It's more they just need hooks in them.

Re: [Coding style] Use hooks

Reply #5 – April 04, 2014, 02:43:31 pm

Well, "it sounds" is not "it is".

The current code related to drafts involved in the presentation of the post page is:

Code: [Select]

		// Are post drafts enabled?
		$context['drafts_save'] = !empty($modSettings['drafts_enabled']) && !empty($modSettings['drafts_post_enabled']) && allowedTo('post_draft');
		$context['drafts_autosave'] = !empty($context['drafts_save']) && !empty($modSettings['drafts_autosave_enabled']) && allowedTo('post_autosave_draft');

Code: [Select]

		// Build a list of drafts that they can load into the editor
		if (!empty($context['drafts_save']))
		{
			$this->_prepareDraftsContext($user_info['id'], $topic);

			if (!empty($context['drafts']))
				$template_layers->add('load_drafts', 100);
		}

To make it use the hooks would be enough to:
1) attach a function to the hook (it should be possible to use integrate_action_post_after),
2) write a function the does the above stuff (and of course move _prepareDraftsContext to another class).
That's all.
So, somewhere (likely in the pre_dispatch) would be enough to have something like:

Code: [Select]

if ($drafts_enabled) // $drafts_enabled doesn't exist at the moment, it's just a placeholder
	add_integration_function('integrate_action_post_after', 'load_post_drafts', false);

Re: [Coding style] Use hooks

Reply #6 – April 04, 2014, 03:40:21 pm

Sounds great. Do it. But, I would first implement a better event system.

Re: [Coding style] Use hooks

Reply #7 – April 04, 2014, 03:49:45 pm

Sorry, by the time I got to writing my reply my brain had turned "implement the draft system through hooks" to "redo the entire post function through hooks" meaning that you'd have to clean up all the what are they doing and what permissions do they have etc. etc. etc.

Re: [Coding style] Use hooks

Reply #8 – April 04, 2014, 04:01:15 pm

@groundup you are most likely right... (I still have to rebase your template_call branch)

@scripple lol no problem. My description was not exactly clear as well, let's say the idea would be: any page should do one thing, anything that could be optional should be implemented via the hooking/eventing system. So, loading drafts for the post page (maybe the answers too), in the Display, let's say all the buttons that are not quote, quick edit and modify (well, okay some moderation may stay as well, but not un/approve

) and so on.
Of course this is just the thought of few seconds, so not many details.
And as pointer out by groundup it may not even be smart to do it now... but IMHO is important start thinking in that terms, so that the moment the code is "ready to go" we will be able to do it without much pain.

Re: [Coding style] Use hooks

Reply #9 – April 04, 2014, 04:57:13 pm

Hey question: if you're going to refactor lotsa stuff to use hooks, is this going to happen between 1.0 RC1 and Final?

Re: [Coding style] Use hooks

Reply #10 – April 04, 2014, 05:05:58 pm

LOL

1.1 maybe.
2.0 more likely.

Quote from: emanuele – April 04, 2014, 04:01:15 pmAnd as pointer out by groundup it may not even be smart to do it now...

Now the question of the day:

Quote from: emanuele – April 04, 2014, 04:01:15 pmbut I'm right, your wrong.

WTF that means? LOL

Re: [Coding style] Use hooks

Reply #11 – April 04, 2014, 05:08:58 pm

It means you like fish fingers and custard.

Re: [Coding style] Use hooks

Reply #12 – April 05, 2014, 10:19:52 am

Spuds used many strange strings in the profanity filter

Re: [Coding style] Use hooks

Reply #13 – April 05, 2014, 04:08:55 pm

Really? I hadn't noticed.

Re: [Coding style] Use hooks

Reply #14 – May 15, 2014, 02:36:56 am

Just a couple of days ago I was thinking that one good candidate for this stuff is menus.
At the moment we have rather big arrays holding lots of stuff in few places (admin, profile, subs, moderation), splitting up these arrays into manageable chunks would probably help readability.
One doubt I have about that is: should we do it the "hard way" or the "soft way"?
The "hard way" means split the menus up in terms of functionality and the spread each piece into its own file, then grab the file and load the menu. That would mean create a specific type of files, otherwise each time someone access the profile, Elk would have to include all the profile-related files (that means quite a bit of code).
The "soft way", instead, would be to just split the menu in functions and keep the functions into the same controller (e.g. Profile.controller.php).
As first step I would use the "soft way", so that everything is still more or less in the same place.