[Phpmyadmin-devel] Refactoring: Displaying query results

Chanaka Dharmarathna pe.chanaka.ck at gmail.com
Fri May 11 14:47:45 CEST 2012 

On Fri, May 11, 2012 at 4:18 PM, Rouslan Placella <rouslan at placella.com>wrote:

> On 11/05/12 08:26, Chanaka Dharmarathna wrote:
> > On Fri, May 11, 2012 at 11:12 AM, Chanaka Dharmarathna<
> > pe.chanaka.ck at gmail.com>  wrote:
> >
> >>
> >>
> >> On Fri, May 11, 2012 at 7:05 AM, Chanaka Dharmarathna<
> >> pe.chanaka.ck at gmail.com>  wrote:
> >>
> >>>
> >>>
> >>> On Fri, May 11, 2012 at 12:39 AM, Rouslan Placella<
> rouslan at placella.com>wrote:
> >>>
> >>>> On 09/05/12 21:08, Chanaka Dharmarathna wrote:
> >>>>> On Wed, May 9, 2012 at 2:27 PM, Rouslan Placella<
> rouslan at placella.com
> >>>>> wrote:
> >>>>>
> >>>>>> On 09/05/12 09:39, Chanaka Dharmarathna wrote:
> >>>>>>> On Wed, May 9, 2012 at 9:21 AM, Chanaka Dharmarathna<
> >>>>>>> pe.chanaka.ck at gmail.com>    wrote:
> >>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Wed, May 9, 2012 at 1:19 AM, Rouslan Placella<
> >>>> rouslan at placella.com
> >>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> On 08/05/12 20:32, Chanaka Dharmarathna wrote:
> >>>>>>>>>> On Tue, May 8, 2012 at 7:23 PM, Chanaka Dharmarathna<
> >>>>>>>>>> pe.chanaka.ck at gmail.com>     wrote:
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> On Tue, May 8, 2012 at 7:08 PM, Marc Delisle<
> marc at infomarc.info>
> >>>>>>>>>     wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>> Le 2012-05-08 09:25, Chanaka Dharmarathna a écrit :
> >>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> See "Class Variables and Methods" in [0].
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> [0] http://pear.php.net/manual/en/standards.naming.php
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Hi Marc,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I went through that document.
> >>>>>>>>>>>>> If the function is global, it will be like
> >>>>>>>>> 'PMA_getDivForSliderEffect()'
> >>>>>>>>>>>>> And if it is a local function (for particular class), it will
> >>>> be
> >>>>>> like
> >>>>>>>>>>>>> 'getDivForSliderEffect()'.
> >>>>>>>>>>>>
> >>>>>>>>>>>> If this is a public method, yes. If a private one, prefix its
> >>>> name
> >>>>>>>>> with
> >>>>>>>>>>>> an underscore.
> >>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Though I went through that document before this, I couldn't
> get
> >>>>>> that
> >>>>>>>>>>>> point.
> >>>>>>>>>>>>> Thanks for pointing out that.
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> --
> >>>>>>>>>>>> Marc Delisle
> >>>>>>>>>>>> http://infomarc.info
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>
> ------------------------------------------------------------------------------
> >>>>>>>>>>>> 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 at lists.sourceforge.net
> >>>>>>>>>>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>> Got it Marc.
> >>>>>>>>>>> Thanks.
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> --
> >>>>>>>>>>> ____________________________________
> >>>>>>>>>>>
> >>>>>>>>>>> Chanaka Indrajith
> >>>>>>>>>>> Bsc.Computer Engineering Undergraduate
> >>>>>>>>>>> Faculty of Engineering
> >>>>>>>>>>> University of Peradeniya
> >>>>>>>>>>> Sri Lanka
> >>>>>>>>>>> ____________________________________
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>> Hi Michal,
> >>>>>>>>>>
> >>>>>>>>>> I just did my first commit to my repository [0].
> >>>>>>>>>> In that commit I have done some refactoring on
> >>>>>>>>> "PMA_displayTableHeaders()"
> >>>>>>>>>> function in "display_tbl.php" file.
> >>>>>>>>>> All HTML renderings by "echo"s were removed and those HTML was
> >>>>>> rendered
> >>>>>>>>> at
> >>>>>>>>>> the bottom of the function at once.
> >>>>>>>>>> As well some related functions in "common.lib.php" file also
> were
> >>>>>>>>> modified
> >>>>>>>>>> in order to return a string of HTML content.
> >>>>>>>>>> But the tests were not yet modified according to my
> modifications.
> >>>>>>>>>> Since there was a conflict after my first local commit [1] (git
> >>>>>>>>> commit), I
> >>>>>>>>>> had to resolve conflicts and commit again. (that is [0])
> >>>>>>>>>>
> >>>>>>>>>> I have already installed phpUnit framework.
> >>>>>>>>>> It will be very helpful if you can suggest me some documentation
> >>>> to
> >>>>>>>>> setup
> >>>>>>>>>> and run tests in PMA (selenium etc).
> >>>>>>>>>>
> >>>>>>>>>> I'm intending to do the above refactoring procedure to all the
> >>>>>>>>> functions in
> >>>>>>>>>> "display_tbl.lib.php" file before moving to any other task.
> >>>>>>>>>>
> >>>>>>>>>> [0] :
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>
> https://github.com/Chanaka/phpmyadmin/commit/dfc73d3494445ff9430e5d0b4f33f800c6e480e0
> >>>>>>>>>> [1] :
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>
> https://github.com/Chanaka/phpmyadmin/commit/004f4f5258a0820975e2da9fa02a6d3ba6622368
> >>>>>>>>>>
> >>>>>>>>>> Regards !
> >>>>>>>>>
> >>>>>>>>> I had a very quick look at your commit. I think that you're
> >>>> indenting
> >>>>>>>>> some lines incorrectly (but I stand to be corrected on this one).
> >>>>>> AFAIK,
> >>>>>>>>> you don't need to line up the operators, just indent by 4 spaces.
> >>>>>> Example:
> >>>>>>>>>
> >>>>>>>>> // THIS IS BAD
> >>>>>>>>> $radio_html .= '<label for="' . $html_field_id . '">'
> >>>>>>>>>                . $choice_label
> >>>>>>>>>                . '</label>';
> >>>>>>>>>
> >>>>>>>>> // THIS IS GOOD
> >>>>>>>>> $radio_html .= '<label for="' . $html_field_id . '">'
> >>>>>>>>>        . $choice_label
> >>>>>>>>>        . '</label>';
> >>>>>>>>>
> >>>>>>>>> // THIS IS ALSO GOOD
> >>>>>>>>> $radio_html .= '<label for="' . $html_field_id . '">';
> >>>>>>>>> $radio_html .= $choice_label;
> >>>>>>>>> $radio_html .= '</label>';
> >>>>>>>>>
> >>>>>>>>> Bye,
> >>>>>>>>> Rouslan
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>
> ------------------------------------------------------------------------------
> >>>>>>>>> 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 at lists.sourceforge.net
> >>>>>>>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Hi Rouslan,
> >>>>>>>>
> >>>>>>>> Thanks for your suggestion.
> >>>>>>>> I'll follow the way you mentioned as "THIS IS GOOD".
> >>>>>>>>
> >>>>>>>> Regards !
> >>>>>>>>
> >>>>>>>> --
> >>>>>>>> ____________________________________
> >>>>>>>>
> >>>>>>>> Chanaka Indrajith
> >>>>>>>> Bsc.Computer Engineering Undergraduate
> >>>>>>>> Faculty of Engineering
> >>>>>>>> University of Peradeniya
> >>>>>>>> Sri Lanka
> >>>>>>>> ____________________________________
> >>>>>>>>
> >>>>>>>>
> >>>>>>> Hi Michal,
> >>>>>>>
> >>>>>>> In some functions there are pure HTML code snippets. (Ex :-
> >>>>>>> PMA_displayTableNavigationOneButton() function in
> >>>> display_tbl.lib.php)
> >>>>>>>
> >>>>>>> As I feel, its not good to convert all these snippets as PHP string
> >>>> and
> >>>>>>> render them. So I'm intending to get those pure HTML snippets into
> >>>>>> separate
> >>>>>>> function. That function can be named as
> >>>> 'PMA_renderTableNavigationForm'
> >>>>>>> (for above case).
> >>>>>>> What do you think about this ?
> >>>>>>>
> >>>>>>> As well is there any reason for the phrase 'One' in the function
> >>>>>>> "PMA_displayTableNavigation*One*Button()" ?
> >>>>>>>
> >>>>>>> Regards !
> >>>>>>
> >>>>>> I think that the above example is *exactly* the kind of stuff that
> >>>> needs
> >>>>>> to be converted to strings.
> >>>>>>
> >>>>>> Bye,
> >>>>>> Rouslan
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>
> ------------------------------------------------------------------------------
> >>>>>> 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 at lists.sourceforge.net
> >>>>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
> >>>>>>
> >>>>>
> >>>>> Hi Rouslan,
> >>>>>
> >>>>> I have little doubt of the way I understand your point.
> >>>>>
> >>>>> You are suggesting to render HTML content by PHP string, instead of
> >>>> using
> >>>>> pure HTML, though there is nicely placed HTML code snippets. (around
> >>>> 5, 6
> >>>>> lines)
> >>>>> Am I correct Rouslan ? Sorry if I'm repeat the wheel.
> >>>>
> >>>> Yes, that's correct. And I don't thing that those lines are nice at
> all.
> >>>>
> >>>> Bye,
> >>>> Rouslan
> >>>>
> >>>>
> >>>>
> ------------------------------------------------------------------------------
> >>>> 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 at lists.sourceforge.net
> >>>> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
> >>>>
> >>>
> >>> Hi Rouslan,
> >>>
> >>> I was already thinking of do the same thing you pointed.
> >>> It will cause some sort of problems if I do the previous thing I
> >>> suggested.
> >>> Thanks for your help.
> >>>
> >>>
> >>> Regards !
> >>> --
> >>> ____________________________________
> >>>
> >>> Chanaka Indrajith
> >>> Bsc.Computer Engineering Undergraduate
> >>> Faculty of Engineering
> >>> University of Peradeniya
> >>> Sri Lanka
> >>> ____________________________________
> >>>
> >>>
> >> Hi Michal,
> >>
> >> The function "PMA_displayTableBody()" in display_tbl.lib.php is always
> >> return true.
> >> But it does not seems like, useful for other place where it have been
> >> used. (Used only in "PMA_displayTable()" function in same file)
> >>
> >> Inside "PMA_displayTable()" function it just assign the return value
> >> (true) to variable called '$clause_is_unique'.
> >> And variable is only used at one place. it is,
> >>
> >> echo '<input type="hidden" name="clause_is_unique"'
> >>              .' value="' . *$clause_is_unique* . '" />' . "\n";
> >>
> >> So, if this always return true, why don't we hard code true in the above
> >> case.
> >> And modify the "PMA_displayTableBody()" function to return string of
> html
> >> content.
> >>
> >> Am I going wrong ?
> >>
> >>
> >> Regards !
> >> --
> >> ____________________________________
> >>
> >> Chanaka Indrajith
> >> Bsc.Computer Engineering Undergraduate
> >> Faculty of Engineering
> >> University of Peradeniya
> >> Sri Lanka
> >> ____________________________________
> >>
> >>
> > Hi Michal,
> >
> > I think I was wrong.
> > Though the block comments of PMA_displayTableBody function, mentioned
> that
> > it always return true,
> > when digging the code, and looking at the bottom comments, seems like its
> > very important.
> >
> > I'll modify the function to return an array, having two elements which
> are
> > string of HTML content and boolean ($clause_is_unique)
> > The name will change from "PMA_displayTableBody" to
> > "PMA_getTableBodyParams".
> > I hope to see your suggestions.
> >
> > Regards !
>
> It looks to me like you could easily get a value for $clause_is_unique
> outside of that function, so that you can leave its name and purpose as
> is. And you'll be able return a single string from it.
>
> Bye,
> Rouslan
>
>
> ------------------------------------------------------------------------------
> 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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>

Hi Rouslan,

You talking about the function "PMA_getUniqueCondition" in "common.lib.php
file" right.
Exactly, I can directly get the value for $clause_is_unique from the output
array of that function.
I'll do the necessary modifications.
Thanks for pointing out that.

Regards !
-- 
____________________________________

Chanaka Indrajith
Bsc.Computer Engineering Undergraduate
Faculty of Engineering
University of Peradeniya
Sri Lanka
____________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20120511/c7b7996a/attachment.html>

More information about the Developers mailing list