Re: [esf-dev] Considerations about Coding Rules

[Jonathan DUMONT <jonathan.dumont@xxxxxxxxxxx>]

Hi everybody,

To answer to these points, here is my point of view :

1-     [NAME-17] The first letter of a variable name is normalised to represent its visibility.
It's true that the Eclipse IDE helps you by coloring the variable differently. But the code may be read with other tools, for example during a text comparison, or a review. Moreover, using a prefix with the variable scope avoid some frequent bugs like the assignation of a parameter and a local variable :

boolean myVar = false;
myfunction(boolean myVar) {
    myVar = myVar
}

This imply to use a 'this' qualifier, which is cumbersome from my point of view.
Moreover, the prefix 'S' for our type in the metamodel should be used to distinguish our meta-objects but not to name our instance and variable. And even if this can be the case, very few of same will be static I think.


2 -  [PRES-13] The cases of a switch instruction are located at the same indentation level than the switch
I think that this is the common way to do (not sure ...), like :
switch ...
case :
    int v = 0;
    break;
...

3 -  [PRES-16] The declarations extends, implements and throws must be located on a separated line, indented as the body of the related Class or method.
Once again, this is to ease the work independently from your screen or IDE. Moreover, this can be useful to distinguish the 'implements' and 'throws' declarations.


4 - [PRES-19] A single empty line must be inserted: Between a method declaration and its first instruction.
You're right ! I think this one is deprecated and must not be kept.

5 - [PRES-19] Two empty lines must be inserted: Between the different sections of a source file (declaration section, properties section, methods section, etc.).
Ok, it can be difficult to maintain. Let's cancel this one.

6 -  [PRES-25] A space must be inserted after a cast instruction.
For this I'm quite sure that it's the standard way to do :
Object myObject = new Integer();
((Integer) myObject).pouet();
And not : ((Integer)myObject).pouet();

7 -  [TIPS-33] A method must contain at most one return instruction.
This is an important point. I really prefer to have only one return by method. The problem of the method length is not really an argument, as a method should not be longer than 50 lines ... and it must be 'coverable' by unit testing. Using several 'return' is like including 'break' and goto instructions. It complexify the maintenance and the debugging.

8 - [ARCH-10] The utilization of “re-export”.
Your example is clearly the good argument to reexport. In OSGI, if you have A <- B <- C and A <- C, this second dependency is meaningless, as to be able to run C, you will need B, and to run B you will need A. That's why using a reexport will simplify the dependencies tree and avoid any 'spaghetti'

9 - [ARCH-13] EMFPluging
AbstractUIPlugin is dependant of Eclipse. Using the EMF plugin implementation allows to be independent from Eclipse and rely on OSGi mechanisms for example if we want to launch any treatment in command line, etc. This doesn't change a lot of thing for the developers.

We can discuss these points next week =)

Regards,

Jonathan

Le 27/08/2015 16:16, MUNOZ JULHO Yupanqui a écrit :

Hi everybody,

 

I have some considerations to do about the coding rules:

 

1-     [NAME-17] The first letter of a variable name is normalised to represent its visibility.

For me it is not necessary, because the Eclipse editor shows to users automatically the different among the variable’s kinds. And another reason is that the prefix are used by classes, like “E” by ECore, “S” by ESF, etc. Consequently if I define a static variable of type “SBlock”, I shall define like “sSBlock”. I think it can create a little bit of confusion.

2-     [PRES-13] The cases of a switch instruction are located at the same indentation level than the switch

I don’t like. This rule is opposite to the current practices, isn’t?

3-     [PRES-16] The declarations extends, implements and throws must be located on a separated line, indented as the body of the related Class or method.

This rule creates unnecessarily line. Nowadays, the users  frequently own the wide screen ( I think).

4-     [PRES-19] A single empty line must be inserted: Between a method declaration and its first instruction.

This rules creates unnecessarily line.

Jonathan, I see you code and, if I understand this rule, you don’t use it.

5-     [PRES-19] Two empty lines must be inserted: Between the different sections of a source file (declaration section, properties section, methods section, etc.).

I think that added comments can do better this job. Add two empty lines can create unnecessarily complexity.

6-     [PRES-25] A space must be inserted after a cast instruction.

I don’t like. This rule is opposite to the current practices, isn’t?

7-     [TIPS-33] A method must contain at most one return instruction.

I don’t agree! In a method with numerous lines, it is more easy to identify a return in the middle of the method and not be forced read all lines for following a variable, because I don’t sure if it may be modified.

8-     [ARCH-10] The utilization of “re-export”.

I don’t have the experience to use re-export. But I think that it can create problems. For example, B depends of A, and C depends of B and A. In this case, if I set re-export A in B. When I  set the dependencies of C, I don’t need set A. Consequently, it is not clear that C depends of A. And if A change, what are the consequences?

9-     [ARCH-13] EMFPluging

Why not AbstractUIPlugin proposed automatically by Eclipse ?  

 

 

Best regards,

 

Yupanqui

_______________________________________________
esf-dev mailing list
esf-dev@xxxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://polarsys.org/mailman/listinfo/esf-dev