[Pki-devel] Automatic reformatting and code style
Christina
cfu at redhat.com
Tue Nov 29 01:40:28 UTC 2011
I agree with Ade.
I have been busy with the actual pki implementation and bug fixes and
I'm not sure if I'm supposed to pay attention to this subject (and I
didn't much), but I think it's worthwhile to chime in. I have worked on
the project for maybe 13 or 15 years since my Netscape days, so I may be
partial, but I think it's a good idea to hear out both sides of the debate.
This is not the 1st controversial item that I have observed, and I have
no idea what happened to the other ones in the end.
May I suggest that you guys separate the non-controversial items from
the controversial ones and leave the controversial ones alone until
people debate it out? Perhaps a wiki page to capture all the
controversial issues so for those of us who can't follow the whole email
threads all the time can have a chance to go to the wiki and find out
points from both sides of the debate?
Each side should be able to go to the wiki and put down your points.
Then we vote at some point, hopefully when I'm around?
Is this acceptable?
thanks!
Christina
On 11/28/2011 07:34 AM, Ade Lee wrote:
> I had no idea that the naming of interfaces and member fields would
> generate such spirited debate.
>
> I'm going to play devil's advocate here, and argue FOR keeping the
> convention of prefixing "m" to object fields.
>
> 1. The arguments against Hungarian notation have mostly been made
> against what is called "systems Hungarian" - that is, things that are
> readily checked by a compiler or an IDE.
>
> When I hover over a variable in eclipse, it will tell me the type - so
> there is no need to prefix the variable with that information. Using
> iFoo for an integer is counterproductive for those reasons.
>
> Attaching "m" to an object tells us about scope - which is not displayed
> when I hover over the variable. Sure, I can look up the declaration -
> but with the prefix, I don't have to.
>
> 2. Does the "m" prefix make the code less readable? I don't think so -
> we're not talking about plzFoo here. What it does do, is allow me when
> looking at segment of code - to instantly determine whether the
> variables in the code are local to the function or member variables.
>
> 3. You may want to change the scope of a variable - and this would
> require you to rename the variable. But I would argue that this is a
> conscious decision - and one that will likely require the
> addition/removal of getters/setters. Having the variable prefixed by
> "m" requires you to think about that. And as has been mentioned,
> renaming is trivial in eclipse.
>
> 4. Renaming a specific variable is trivial in eclipse, but I have not
> found a simple way to do a mass renaming so that all the variables that
> are now in Hungarian notation in eclipse. There is a way to enforce the
> use of a prefix - but not a way to systematically remove the prefixes.
>
> If we can't do that, then we run the risk of having code which conformed
> to old standard - along with code which conformed to a new standard
> which did not require the "m" - which I would argue is much worse.
>
> 5. Even if we could remove all Hungarian notation, this would make
> merges from 8.X to the tip much more labor intensive. Its one thing to
> have to do things manually because of formatting. Its another to have
> to replace variables stuck in the code.
>
> Ade
>
> On Wed, 2011-11-23 at 17:50 -0500, Adam Young wrote:
>> vi long since became capable of handling either format. I'll leave it as ana excersize to you vi guys to figure out how to get it to match.
>>
>> Renaming is trivial in Eclipse, and "Its always been done this way" is not sufficient grounds to keep up a bad practice.
>>
>>
>> The code is going to shrink, get reordred, and things are going to get renamed. THis is acleanup that will have a very large net positive advantage, and we have the flexibility to do it right. Lets not hamstring outselves with decisions bad in the past that no longer apply: keep the good, but don't hoard things, esepcially not in code.
>>
>> INterfaces have their uses, but the PKI code base shows some of the mistakes made by following the "best practices" of the day that have since been discarded. Interfaces in Java should be used for cutting dependncies, information hading and the like, but should be the exception, not the rule. A fundamental guideline is "favor collaboration over inheritance" and that means Interfaces as well as abstract base classes. I'm not saying "Don't use them" but I am saying "Be prepared to justify their use" if you do.
>>
>>
>> For the domain model, using an IRequest to front RequestImpl where there is only one Impl is an anti-pattern. a Date transfer object should have minimal functionality in it, and no external dependencies. Thus, it gains no benefit from the abstraction of the Interface.
>>
>> A much better approach is to make Request a POJO, and, if posssible, immutable. I'll go more into how to code this way as examples pop up.
>>
>>
>>
>> Sorry for top posting, but I'm having mail failure and can only use Zimbra web, which doesn't do the> thing.
>>
>>
>> ----- Original Message -----
>> From: "Matthew Harmsen"<mharmsen at redhat.com>
>> To: alee at redhat.com
>> Cc: "Adam Young"<ayoung at redhat.com>, pki-devel at redhat.com
>> Sent: Wednesday, November 23, 2011 5:05:47 PM
>> Subject: Re: [Pki-devel] Automatic reformatting and code style
>>
>> On 11/23/11 11:57, Ade Lee wrote:
>>> I looked at our current guidelines and noticed the following differences
>>> with the Sun guides:
>>>
>>> 1. Placement of braces
>>> In the current coding guidelines, we say classes and methods should look
>>> like this:
>>> public class MyClass
>>> {
>>> ...
>>> }
>>>
>>> instead of this (as in the Sun standard):
>>> public class MyClass {
>>> ...
>>> }
>> The history behind this probably comes from a nice handy 'vi' shortcut
>> (pressing ']' twice) that allows for developers to quickly find the
>> beginning of
>> various 'functions' in 'C'/'C++'; note that the opening brace must be a
>> standalone
>> character in the first column for this to work.
>>
>> This was probably applied to the Java code to find the beginning and
>> ending of a
>> 'class' in java files that contained more than one non-nested class in a
>> single '.java'
>> file, so I don't see that big of a problem in changing this for Java --
>> however,
>> I would prefer that 'C'/'C++' functions/methods retain this handy
>> feature, as
>> not every programmer is particularly fond of IDEs, and I would argue that we
>> should not require developers to use an IDE such as eclipse.
>>
>> That being said, I would prefer multiple non-nested public classes to
>> reside in their
>> own files, leaving only cases where we need/require standalone
>> private/protected
>> classes to co-exist within the same file? I am uncertain if we even
>> have any of these.
>>
>>> 2. We require that interface names begin with "I". Sun says nothing
>>> about this. This is probably a good one to keep.
>> I agree that we should keep this 'handy' notation.
>>> 3. We specify "no tabs". Sun says tabs are optional. We should keep
>>> this.
>> My personal preference is to use 'spaces' instead of 'tabs' primarily
>> due to the
>> variable 'tabstop' settings in files.
>>> 4. We say "Static methods should begin with a capital letter with each
>>> subsequent new word in uppercase, and subsequent letters in each word in
>>> lower case. Sun has no special treatment for static methods - ie. they
>>> are treated just like other methods .. ie. beginning with a lower-case
>>> letter.
>> I have no particular preference one way or another on this.
>>> 5. We do have some guidelines for naming functions that go beyond what
>>> Sun specifies - and also perhaps, beyond what eclipse can verify.
>>>
>>> For example:
>>> * Get and set methods should begin with "get" / "set" and return the
>>> appropriate object type.
>>> * Boolean get methods should use "is" or "can" as a prefix, such as
>>> "isUndoable()" rather than "getUndoable()".
>>> * Factory class names should include the word "Factory". Factory method
>>> names should start with the word "Make."
>>> * Methods for debug-only implementations should begin with "debug".
>>> * Member variables should begin with "m". For example, mMemberVariable.
>> These all seem fine to me.
>>
>> While I understand Adam's doesn't like Hungarian notation, the
>> downside of renaming everything will make it far more difficult
>> for the people who are the most familiar with this code to
>> continue to make changes.
>>
>>> My take on this is that we should adopt the Sun coding standards and add
>>> the additional requirements that make sense - like the ones listed in
>>> point 5 above. For the cases where we conflict with the Sun standards,
>>> we should go with the Sun standards instead.
>>>
>>> Comments?
>>> Ade
>>>
>>> On Wed, 2011-11-23 at 12:26 -0500, Adam Young wrote:
>>>> MIght I highly encourage that we folow the Sun guides, as it is the Inustry standard in Java, and it is pretty staightforward.
>>>>
>>>> ----- Original Message -----
>>>> From: "Ade Lee"<alee at redhat.com>
>>>> To: pki-devel at redhat.com
>>>> Sent: Wednesday, November 23, 2011 10:48:42 AM
>>>> Subject: [Pki-devel] Automatic reformatting and code style
>>>>
>>>> Hi all,
>>>>
>>>> It has been decided that the code should go through an automatic
>>>> reformatting on the trunk to ensure that everything matches the
>>>> project's coding standards.
>>>>
>>>> Prior to this, we need to review the coding standards and confirm that
>>>> they are what we want to use.
>>>>
>>>> The current coding standards for the project are referenced here:
>>>> http://pki.fedoraproject.org/wiki/PKI_C_Coding_Style
>>>> http://pki.fedoraproject.org/wiki/PKI_Java_Coding_Style
>>>>
>>>> Some alternative styles:
>>>> http://freeipa.org/page/Coding_Style (C)
>>>> http://www.oracle.com/technetwork/java/codeconvtoc-136057.html (java,
>>>> sun conventions)
>>>>
>>>> We should focus on the java coding style first, followed by C. Most of
>>>> the Perl code is mostly going away most likely, so no need to focus on
>>>> that.
>>>>
>>>> IPA has a style guide for python, which, unless we have another
>>>> compelling reason, we should probably use that:
>>>>
>>>> http://freeipa.org/page/Python_Coding_Style
>>>>
>>>> We'd like to get this resolved soon - so as not to obscure any future
>>>> changes as we do new development. So, please devote some attention to
>>>> this soon.
>>>>
>>>> Thanks,
>>>> Ade
>>>>
>>>> _______________________________________________
>>>> Pki-devel mailing list
>>>> Pki-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/pki-devel
>>> _______________________________________________
>>> Pki-devel mailing list
>>> Pki-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/pki-devel
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5130 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20111128/c1945a7a/attachment.p7s>
More information about the Pki-devel
mailing list