Skip to main content
Bits from the code refactoring zone (there be lions) Started by TestMonkey · · Read 21032 times 0 Members and 1 Guest are viewing this topic. previous topic - next topic

Bits from the code refactoring zone (there be lions)

It's quite a while since we are hard at work on this code, and we haven't updated you all with what's up. Let me try to fix that now, since coffee is coming and there's no better time for stories than a rainy morning.

The code has seen and is seeing a relatively heavy code rework, of several kinds.
 # code-level refactoring
 # architecture-level refactoring
 # rewrites
 # other
Apart from them new features beasts, which are really the realm of Feature Cat, and I won't address them here.

By code-level refactoring, I simply refer to all the code improvements such as localized rewrites of a list to use a standard generic list, or simplifications of core/template code in an area, or work to make place for hooks or insertions of add-ons/theme makers - or many others. Very important for everything, general code design and cleanliness, easier to work with, easy to debug.

By architecture-level refactoring/rework, I mean in particular the work to bring the software to a MVC-type of architecture. We have documented how the software and the execution flow are structured, on the wiki:
https://github.com/elkarte/Elkarte/wiki/Architecture
https://github.com/elkarte/Elkarte/wiki/Mvc

Where are we on these? Well, let me try. (coffee first though!)

# Admin controllers
Most are so cool. :)
Take a look at ManageBans for example. You'll recognize it, it's yer old beast somehow. (though off-hand I think most code from 2.0 has changed). But we have redesigned it in an object oriented way, with clear, single-purpose actions handlers, respecting naming patterns. Ema has entirely rewritten its code, within his bans pet branch, of which the smallest example is them lists: using genericlists is much more fun!. TE has refactored its queries and sub functions, fixed and brought the class up to standards.
Many admin controllers are much better now. Off hand, I think that I should update the dispatching mechanism (it's behind them times).

 # Moderation controllers, subs, menu
 # Profile, controllers, subs, menu
 # Personal messages, controllers, subs, menu
...Mmm, well. Some of them are quite messy. We have some improvements comparing to SMF 2.1, but the areas are a challenge of their own. They now have the basic OO wrappers around (but that says nothing yet, unless it sayz it looks ugly atm :P). Probably these will see a few rounds of rework, to move queries .subs files, to refactor the code, the menus, the calls, the actual actions to execute. Moderation code is is also embedded in more of the rest of the code than others; it could benefit from a looser coupling with the rest.

 # User-actions controllers (apart from the above):
It depends. Some are nice, i.e. Karma. Err, well, bad example, cuz we have no love for it. Then my friend Reminder. Or, Announce isn't fine but we can bring it there quickly enough.
Some are very messy still. (hello, action_post()).

 # .subs
The codebase has seen moves of queries from controllers to .subs functions, refactoring of a few queries into a more reusable function for .subs, discovery of duplicate queries in code and replacement with calls to the respective now existing little function, stuffs like that.
Simply moving queries out of controllers (in their own .subs function) is a gain. For controller readability at a glance, to be reused if needed some other time, and to have more 'examples' of needed functionality in .subs, which at their turn may be later reimplemented in a better way.
Ema has made more refactoring, creating some new parameterized functions, more reusable, now available for use for any client code, add-ons included.
We're still to get to some others, such as more complex queries in personal messages, search, or moderation stuffs. They're already nice little monsters composed from bits, probably they can only be moved in their own place, and not get more reusable than that. That's fine, they're there to do a job and do it well.

 # Templates
A few were killed (quite mercilessly, I must add). I think I slightly updated a few sub-templates (to be more inline with controller changes). You got me there though, I didn't deal much with them shiny stuffs, better ask them other meanies what do they do to them.

I guess ideally, a controller action_ should have a corresponding template_ (well if it haz any at all), or more pieces.

 # Rewrites
Some of the work on the code is a rewrite. It's entirely changing the code, design wise and code-level. Some implementations (i.e. a function now using lists) are rewriting the thing.
Some of the code needs it. Some other of the code is an example of how more can be written. ;)

 # Other, i.e. database layer.
We are replacing the database layer with an object oriented one. At least, that'll make it more 'natural' to use than functions called through an array. It'll also allow more flexibility to reimplement under wraps, with no or little changes to the rest.

No more coffee, so I'd better stop. Just to give you a quick view of sorts.
Last Edit: May 16, 2013, 02:07:37 am by TestMonkey
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: Bits from the code refactoring zone (there be lions)

Reply #1

Interesting, very interesting.