On 07/05/12 09:27, Chanaka Dharmarathna wrote:
On Sat, May 5, 2012 at 1:16 AM, Chanaka Dharmarathna< pe.chanaka.ck@gmail.com> wrote:
On Thu, May 3, 2012 at 4:38 PM, Michal Čihařmichal@cihar.com wrote:
Hi
Dne Tue, 1 May 2012 02:02:38 +0530 Chanaka Dharmarathnape.chanaka.ck@gmail.com napsal(a):
I'm identifying the refactoring points in 'display_tbl.lib.php' file. I
am
trying to break the tasks on following areas.
- As a library file this should not render the HTML codes in functions
it
selves. First, all the HTML content in functions stored in string and render at the bottom of the function. Next, return the string having the HTML content, instead of rendering it inside the function itself and, from the calling end render that
content.
Probably method names will be modified with adding 'get' phrase.
- Some naming conventions are used incorrectly. For method name in lib
file, naming convention is used generally used as 'PMA_methodName'. But some are not following this. While correcting them, suggested conventions in my proposal [0] is
going to
applied.
- There are several functions having more than 200 lines. I'm intending
to
divide those functions to smaller ones. (PMA_displayTableNavigation(), PMA_displayTableHeaders(), PMA_displayTableBody(), PMA_displayVerticalTable(), PMA_displayTable())
I think all this should be rather wrapped in object, so it would be PMA_Display::someFunction.
-- Michal Čihař | http://cihar.com | http://blog.cihar.com
Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
Hi Michal,
Thanks for your reply. I got the point Michal. After the mentioned tasks plus making this as a class, I can use 'PMA_Display::someFunction' where needed.
Regards !
Chanaka Indrajith Bsc.Computer Engineering Undergraduate Faculty of Engineering University of Peradeniya Sri Lanka ____________________________________
Hi Michal,
I am involving with some refactoring in "PMA_displayTableHeaders" function in "display_tbl.lib.php" file. As I mentioned in my previous mail, first I'm trying to render all HTML at the bottom of the function itself. But I'm facing some problems.
While current code rendering HTML codes using "echo", there are some function calls which does not return anything but render HTML inside that. Ex : "PMA_generate_slider_effect" function in common.lib.php file -> call by 739 line in "display_tbl.lib.php" file.
There are several situations like that, in this function. Since they have used in other files also (test cases also implemented for them), I do not think to change those functions.
What I'm going to do is, remove HTML renderings everywhere inside the function, as well as possible by assigning those HTML to a variable, and render existing content in the variable before calling that kind of function (as above mentioned).
Ex : func { $tableHeadersHTML .= ''; ---- ---- $tableHeadersHTML .= '<div>' . 'some html content'; ---- *echo $tableHeadersHTML; // render HTML content before another function call which not just return a string of HTML* PMA_generate_slider_effect(params); ---- // repeat the procedure }
Before doing this, I would like to hear your suggestions on this.
Regards !
I think that you should rewrite the PMA_generate_slider_effect function to output a string, like it should. I'd say don't worry about breaking test cases for functions that do things the wrong way around, besides those should simple enough to fix. As far as spotting other occurrences of the function in the code base, there are only 4 of them in total (and that includes the function declaration), so there is no excuse not to fix all of them. To find every occurrence, run:
git grep PMA_generate_slider_effect
Bye, Rouslan