[jitsi-dev] code style


#1

https://jitsi.org/Documentation/CodeConvention

This stands out:

   Use package imports (package.*) and DO NOT import classes themselves.

FindBugs actually complains about this practice. Personally, I don't
think this should be as rigorously enforced as something like indentation.

In reSIProcate SVN, we have a code style preferences file that Eclipse
users can import to quickly get the right settings. Does something like
that exist for Jitsi?


#2

I also don't like all of it, but it simply should stay as the existing code
base - which adheres to all these rules.
There's a formatter template here:
http://www.jitsi.org/wiki/pub/sip-communicator/Formatter

Ingo

···

-----Original Message-----
From: dev-bounces@jitsi.org [mailto:dev-bounces@jitsi.org] On Behalf Of
Daniel Pocock
Sent: Montag, 7. Juli 2014 16:06
To: dev@jitsi.org
Subject: [jitsi-dev] code style

https://jitsi.org/Documentation/CodeConvention

This stands out:

   Use package imports (package.*) and DO NOT import classes themselves.

FindBugs actually complains about this practice. Personally, I don't think
this should be as rigorously enforced as something like indentation.

In reSIProcate SVN, we have a code style preferences file that Eclipse users
can import to quickly get the right settings. Does something like that
exist for Jitsi?

_______________________________________________
dev mailing list
dev@jitsi.org
Unsubscribe instructions and other list options:
http://lists.jitsi.org/mailman/listinfo/dev


#3

I don't think that is applicable for everything

Some things (like the position of the "{" on the next line) means
nothing to the compiler. I'm not complaining about all those types of
things.

However, "import foo.*" is not functionally the same as "import
foo.Bar". It is not a formatting issue. The appearance of extra import
statements at the top of the code doesn't impact the readability of the
code in any way. Some IDEs even hide the import statements. So my
suggestion would be to encourage the latter syntax for new contributions.

···

On 07/07/14 11:27, Ingo Bauersachs wrote:

I also don't like all of it, but it simply should stay as the existing code
base - which adheres to all these rules.


#4

It actually is a matter of formatting because the Java compiler would
convert asterisk imports to class imports and compile both notations
to the same byte code.

As Ingo already pointed out, there are already 700 000 lines of code
written in that convention so we are very unlikely to change it given
that all the pros and cons are a matter of personal preference (and
yes, I do prefer asterisk imports :wink: )

Emil

···

On Wed, Jul 9, 2014 at 2:28 PM, Daniel Pocock <daniel@pocock.pro> wrote:

On 07/07/14 11:27, Ingo Bauersachs wrote:

I also don't like all of it, but it simply should stay as the existing code
base - which adheres to all these rules.

I don't think that is applicable for everything

Some things (like the position of the "{" on the next line) means
nothing to the compiler. I'm not complaining about all those types of
things.

However, "import foo.*" is not functionally the same as "import
foo.Bar". It is not a formatting issue.

--
https://jitsi.org


#5

Technically, there is a difference. For example, if package org.foo.baseclasses provides a class "Address" and org.foo.sip provides a class "Address" (which perhaps extends that class), then "import org.foo.baseclasses.*" and "import org.foo.sip.* " will cause problems when explicit references to Address class are used "error: reference to Address is ambiguous"

For this reason I think import foo.* is less desirable.

···

On 07/09/2014 09:23 AM, Emil Ivov wrote:

On Wed, Jul 9, 2014 at 2:28 PM, Daniel Pocock <daniel@pocock.pro> wrote:

On 07/07/14 11:27, Ingo Bauersachs wrote:

I also don't like all of it, but it simply should stay as the existing code
base - which adheres to all these rules.

I don't think that is applicable for everything

Some things (like the position of the "{" on the next line) means
nothing to the compiler. I'm not complaining about all those types of
things.

However, "import foo.*" is not functionally the same as "import
foo.Bar". It is not a formatting issue.

It actually is a matter of formatting because the Java compiler would
convert asterisk imports to class imports and compile both notations
to the same byte code.

As Ingo already pointed out, there are already 700 000 lines of code
written in that convention so we are very unlikely to change it given
that all the pros and cons are a matter of personal preference (and
yes, I do prefer asterisk imports :wink: )

--
Thanks,
David Mansfield
Cobite, INC.


#6

I also don't like all of it, but it simply should stay as the existing code
base - which adheres to all these rules.

I don't think that is applicable for everything

Some things (like the position of the "{" on the next line) means
nothing to the compiler. I'm not complaining about all those types of
things.

However, "import foo.*" is not functionally the same as "import
foo.Bar". It is not a formatting issue.

It actually is a matter of formatting because the Java compiler would
convert asterisk imports to class imports and compile both notations
to the same byte code.

That is not strictly true - the same compiler/same Java version, same
libraries will resolve the same classes when it sees asterisks.

When a new Java version appears or a new third party library is updated,
it may include additional classes that didn't exist before, having the
same names as classes in other packages, creating ambiguity. In that
future scenario, the compile will break with something like "reference
to Foo is ambiguous, both class bar.Foo in bar and class foo.Foo in foo
match". Obviously this is not so hard to fix when it happens.

Being more pedantic, if a deprecated class goes away but some other
class appears in another package with the same name then there will be
unpredictable results. The chance of this happening is obviously small,
but the effort to investigate and resolve such an issue could be quite
high, because there may not be obvious clues what is wrong.

That is why I suggest this is not just a code formatting issue.

As Ingo already pointed out, there are already 700 000 lines of code
written in that convention so we are very unlikely to change it given
that all the pros and cons are a matter of personal preference (and
yes, I do prefer asterisk imports :wink: )

I'm not suggesting there should be any effort to retrospectively change
this for existing code.

···

On 09/07/14 15:23, Emil Ivov wrote:

On Wed, Jul 9, 2014 at 2:28 PM, Daniel Pocock <daniel@pocock.pro> wrote:

On 07/07/14 11:27, Ingo Bauersachs wrote:


#7

Indeed, and we do authorise use of class imports when that is the case
(there are already a number of cases in the code).

Emil

···

On Wed, Jul 9, 2014 at 6:33 PM, David Mansfield <jitsi@dm.cobite.com> wrote:

On 07/09/2014 09:23 AM, Emil Ivov wrote:

On Wed, Jul 9, 2014 at 2:28 PM, Daniel Pocock <daniel@pocock.pro> wrote:

On 07/07/14 11:27, Ingo Bauersachs wrote:

I also don't like all of it, but it simply should stay as the existing
code
base - which adheres to all these rules.

I don't think that is applicable for everything

Some things (like the position of the "{" on the next line) means
nothing to the compiler. I'm not complaining about all those types of
things.

However, "import foo.*" is not functionally the same as "import
foo.Bar". It is not a formatting issue.

It actually is a matter of formatting because the Java compiler would
convert asterisk imports to class imports and compile both notations
to the same byte code.

As Ingo already pointed out, there are already 700 000 lines of code
written in that convention so we are very unlikely to change it given
that all the pros and cons are a matter of personal preference (and
yes, I do prefer asterisk imports :wink: )

Technically, there is a difference. For example, if package
org.foo.baseclasses provides a class "Address" and org.foo.sip provides a
class "Address" (which perhaps extends that class), then "import
org.foo.baseclasses.*" and "import org.foo.sip.* " will cause problems when
explicit references to Address class are used "error: reference to Address
is ambiguous"

--
https://jitsi.org


#8

I also don't like all of it, but it simply should stay as the existing code
base - which adheres to all these rules.

I don't think that is applicable for everything

Some things (like the position of the "{" on the next line) means
nothing to the compiler. I'm not complaining about all those types of
things.

However, "import foo.*" is not functionally the same as "import
foo.Bar". It is not a formatting issue.

It actually is a matter of formatting because the Java compiler would
convert asterisk imports to class imports and compile both notations
to the same byte code.

That is not strictly true - the same compiler/same Java version, same
libraries will resolve the same classes when it sees asterisks.

When a new Java version appears or a new third party library is updated,
it may include additional classes that didn't exist before, having the
same names as classes in other packages, creating ambiguity. In that
future scenario, the compile will break with something like "reference
to Foo is ambiguous, both class bar.Foo in bar and class foo.Foo in foo
match". Obviously this is not so hard to fix when it happens.

Being more pedantic, if a deprecated class goes away but some other
class appears in another package with the same name then there will be
unpredictable results. The chance of this happening is obviously small,
but the effort to investigate and resolve such an issue could be quite
high, because there may not be obvious clues what is wrong.

That is why I suggest this is not just a code formatting issue.

I understand what you mean but I'd rather we do the fixes when
necessary than breaking consistence.

As Ingo already pointed out, there are already 700 000 lines of code
written in that convention so we are very unlikely to change it given
that all the pros and cons are a matter of personal preference (and
yes, I do prefer asterisk imports :wink: )

I'm not suggesting there should be any effort to retrospectively change
this for existing code.

No, of course not, I was referring to the fact that consistence would
be broken (yes, that's purely cosmetic ... and hence of absolute
importance :wink: ).

Emil

···

On Wed, Jul 9, 2014 at 3:47 PM, Daniel Pocock <daniel@pocock.pro> wrote:

On 09/07/14 15:23, Emil Ivov wrote:

On Wed, Jul 9, 2014 at 2:28 PM, Daniel Pocock <daniel@pocock.pro> wrote:

On 07/07/14 11:27, Ingo Bauersachs wrote:

--
https://jitsi.org