[Pki-devel] Automatic reformatting and code style
Adam Young
ayoung at redhat.com
Tue Nov 29 14:22:27 UTC 2011
On 11/28/2011 08:40 PM, Christina wrote:
> 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?
Makes sense, although I think we can talk it out in this forum. Email,
especially a publicly archived list like this, is actually a good way
to drive he discussion, better, I've found, than a wiki.
There are two issues here, though, as Christina points out: the
automatable changes, which should be fairly non-contraversial, and the
ones that will require manual changes. We are not going to automate
the removal of the 'm' friom member variables and the like, so we can
defer making a decision on those.
>
> 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
>
>
>
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20111129/fd78f55f/attachment.htm>
More information about the Pki-devel
mailing list