On coding standards

Richard Newman rnewman at mozilla.com
Thu Feb 27 10:05:05 PST 2014


Broadening the circulation of this to mobile-firefox-dev.

We have approximate consensus within the core frontend team: dropping prefixed names, bracing conditionals, and doing so incrementally as we go.

Ready thyselves!


On  27 Feb 2014, at 9:35 AM, Mark Finkle <mfinkle at mozilla.com> wrote:

> I also agree we should move ahead with the proposed style/structure changes. 
> 
> Change: dropping aFoo prefixed arguments, always bracing conditionals, "use strict" (JS only)
> Where/When: New files and piecemeal on edited files. Any changes in an existing file should be in a separate patch (patch 0). Do not intermingle substantive and style changes in the same patch. I really do not want to see bugs filed with patch 0 only. We still need to focus on getting real feature work completed and not just curb our coding OCD.
> 
> I'm all for the newer style (I've definitely been doing this in my JS), but I don't want us to spend too much time futzing around updating existing code. +1 to using the new style in new files, and I'd support "part 0" clean-up patches in bugs where developers are going to be changing a lot of an existing file anyway (and that's also a good way to audit the exiting code for unnoticed problems).
> 
> Margaret
> 
> ----- Original Message -----
> > From: "Richard Newman" <rnewman at mozilla.com>
> > To: mobile-frontend at mozilla.org
> > Sent: Thursday, February 27, 2014 9:01:37 AM
> > Subject: On coding standards
> > 
> > Over the past year or three, desktop JS has been moving away from the older
> > "toolkit-style" standard: dropping aFoo prefixed arguments, always bracing
> > conditionals, "use strict", etc.
> > 
> > We've also done this to an extent in mobile: new Java code tends not to have
> > aFoo arguments, and tends not to use unbraced conditionals.
> > 
> > The services code in Fennec (about 1/4 of the codebase) already goes all the
> > way, using 'modern' Java coding conventions throughout.
> > 
> > 
> > I don't expect any pushback on always bracing conditionals, and we're already
> > dropping aFoo, so:
> > 
> > I propose eliminating the use of mFoo/sBar in the rest of Fennec. Here's why.
> > 
> > 
> > 1. It doesn't add value, when it's wrong it's misleading, and apparently
> > nobody pays enough attention to it. For example, GeckoProfile contains this
> > line, which wesj wrote in 3c552ee3aa5e, bnicholson reviewed, and nobody
> > spotted for at least 6 months:
> > 
> >     private static GeckoProfile mGuestProfile = null;
> > 
> > (I'm fixing that as part of Bug 707123.)
> > 
> > If wesj and bnicholson -- two of our most awesome crew -- can make this
> > mistake, and nobody spotted it or nobody was affected by it, then why
> > bother?
> > 
> > 
> > 2. It's unnecessarily verbose, considering that it adds no value. At the
> > point of definition we already know it's static: it has the word 'static'
> > next to it. The best you can do is match that, and the worst is to get it
> > wrong! At the point of use, Java already has conventions for being explicit
> > about what you're doing: Classname.staticMember, this.member.
> > 
> > 
> > 3. If you get access wrong, you'll typically get a compiler error. You can't
> > access instance members from a static context. You can access statics in
> > scope (within a class definition) without qualification, and that's OK. If
> > you're worried about getting that wrong, use ClassName.foo instead. The only
> > other case that builds -- accessing a static member via an instance -- is
> > accompanied by a compiler warning and a yellow flag in Eclipse.
> > 
> > 
> > 4. Speaking of Eclipse: IDEs make it obvious what everything is, using text
> > styling, colors, hints, tooltips, and the other affordances available. I
> > acknowledge that this doesn't apply to the process of code review, but I'll
> > be so bold as to assert that the set of bugs that might be caught by saying
> > "waitaminute, that's static!" is small, uncommon, and probably overlaps with
> > the set of bugs that requires you to *understand* the class definition, not
> > just pattern-match.
> > 
> > (No disrespect to pattern-match reviewing: I do it all the time.)
> > 
> > 
> > 
> > My proposal is to ditch this style for new files, fix up old files as we go
> > (just as we do for bracing and broken indents), and enjoy our cheerful new
> > world.
> > 
> > Thoughts?
> > 
> > -R

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/mobile-firefox-dev/attachments/20140227/7d0d2381/attachment.html>


More information about the mobile-firefox-dev mailing list