Refactor toc() and li()
Posted: Thu May 08, 2014 12:00 am
Hello Community,
does anybody understand what's going on inside of toc() and li()?
I pick up on this issue, because I recently commited a fix which introduced the funtion XH_buildHc(); this function is closely related to toc() and $hc is often used in templates as first argument to li() (or rather xli() of xtoc). This pointed my attraction to toc() and li() again, and IMHO both functions are way too long and too cryptic and should be refactored ASAP.
I suggest to apply "Replace Method with Method Object" on both functions first (toc() becoming the model and li() the view), and after having unravelled the algorithms to move related functionality (such as XH_buildHC()) to the respective class(es). Especially, I suggest to keep an eye on potential customizations and to create an API that allows these without restricting them to current functionality. To better understand what I mean, consider the original xtoc: this wasn't able to cater for $cf['menu']['sdoc']="parent", which was introduced with CMSimple 2.8, and had to be replaced by xtoc28--however, existing templates most likely weren't updated, so the new functionality could not be used.
Frankly, I don't know how much flexibility wrt. customization we can achieve, but we should strive for some sensible maximum. After all, I consider xtoc, AdvancedToc etc. merely as workarounds.
As we already have a quite comprehensive test suite regarding toc() and li() in place (see trunk/tests/unit/MenuTest.php), and the current API (toc() and li()) won't change, I dare to propose this refactoring for XH 1.6.3.
Christoph
does anybody understand what's going on inside of toc() and li()?
I pick up on this issue, because I recently commited a fix which introduced the funtion XH_buildHc(); this function is closely related to toc() and $hc is often used in templates as first argument to li() (or rather xli() of xtoc). This pointed my attraction to toc() and li() again, and IMHO both functions are way too long and too cryptic and should be refactored ASAP.
I suggest to apply "Replace Method with Method Object" on both functions first (toc() becoming the model and li() the view), and after having unravelled the algorithms to move related functionality (such as XH_buildHC()) to the respective class(es). Especially, I suggest to keep an eye on potential customizations and to create an API that allows these without restricting them to current functionality. To better understand what I mean, consider the original xtoc: this wasn't able to cater for $cf['menu']['sdoc']="parent", which was introduced with CMSimple 2.8, and had to be replaced by xtoc28--however, existing templates most likely weren't updated, so the new functionality could not be used.
Frankly, I don't know how much flexibility wrt. customization we can achieve, but we should strive for some sensible maximum. After all, I consider xtoc, AdvancedToc etc. merely as workarounds.
As we already have a quite comprehensive test suite regarding toc() and li() in place (see trunk/tests/unit/MenuTest.php), and the current API (toc() and li()) won't change, I dare to propose this refactoring for XH 1.6.3.
Christoph