On Sat, Jul 30, 2011 at 5:22 AM, Dieter Adriaenssens < dieter.adriaenssens@gmail.com> wrote:

2011/7/22 Madhura Jayaratne madhura.cj@gmail.com:

...

Hi all, Looking at the CheckStyle reports from the CI, all the export files show very high counts of indentation rule violations. The code in the 'else' block of 'if (isset($plugin_list))' condition is wrongly indented in all of these files. There are two possible ways to correct this.

1. Correctly indent the 'else' block
2. Add 'return' statement to the end of 'if' block and remove 'else {'

part.

...

Which one if preferred?

Looking at the code (I only checked sql.php and xml.php) I wonder why the different functions are defined within the 'else' block. This doesn't seem right to me. There must be cleaner ways of conditionally defining a function than putting them in an if/else block. I'm not sure even how the php compiler parses this? Anyway I don't think we can rely on the behaviour of the php compiler to not define these functions.

Since we did not have any bug reports about such a problem I believe most of the compilers, if not all, work as expected with the if/else block. But as you mentioned, it would be nice to have a cleaner mechanism to do this.

When I sent the earlier mail I was under the impression that a 'return' statement as the last line of the 'if' block would let me remove the else block, which would correct those CheckStyle violations. I was comparing this to the import plugins. However since export plugins contains methods in the 'else' blocks, this is not possible. It amounts to redefining, already defined methods and I have to give it up.

2011/8/1 Michal Čihař michal@cihar.com:

Hi

Dne Fri, 22 Jul 2011 22:34:55 +0530 Madhura Jayaratne madhura.cj@gmail.com napsal(a):

...

Looking at the CheckStyle reports from the CI, all the export files show very high counts of indentation rule violations. The code in the 'else' block of 'if (isset($plugin_list))' condition is wrongly indented in all of these files.

There are two possible ways to correct this.

1. Correctly indent the 'else' block
2. Add 'return' statement to the end of 'if' block and remove 'else {' part.

Which one if preferred?

I don't think 2 will work, so out of these 1 is only solution. Rewrite it so that same functions are not defined in all plugins (using classes instead) would be preferable solution, though it is much more complex than reindenting :-).

Transforming the ilmport/export modules to Classes would be a nice GSoC project. ;) But probably not sufficient to fill 12 weeks, bu it can be part of a bigger OOP project.

--        Michal Čihař | http://cihar.com | http://phpmyadmin.cz

---

Got Input?   Slashdot Needs You. Take our quick survey online.  Come on, we don't ask for help often. Plus, you'll get a chance to win $100 to spend on ThinkGeek. http://p.sf.net/sfu/slashdot-survey

---

Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel

On Mon, Aug 1, 2011 at 7:03 PM, Michal Čihař michal@cihar.com wrote:

Hi

Dne Fri, 22 Jul 2011 22:34:55 +0530 Madhura Jayaratne madhura.cj@gmail.com napsal(a):

...

Looking at the CheckStyle reports from the CI, all the export files show very high counts of indentation rule violations. The code in the 'else' block of 'if (isset($plugin_list))' condition is wrongly indented in all of these files.

There are two possible ways to correct this.

1. Correctly indent the 'else' block
2. Add 'return' statement to the end of 'if' block and remove 'else {'

part.

...

Which one if preferred?

I don't think 2 will work, so out of these 1 is only solution. Rewrite it so that same functions are not defined in all plugins (using classes instead) would be preferable solution, though it is much more complex than reindenting :-).

Yep, I was comparing it with the import plugins and didn't notice that these contains methods. So what is preferred, reindenting as a quick solution or letting it there as it is till it is rewritten in OOP?