Prefer Explicit Declarations?

[brianmfear]

I have lots of problems with this section, but I'll try to sum them up in this one issue.

First, I'd suggest that private should never appear in code. This is because (a) it's the default model, and (b) it's 8 characters counted against you each time you use it. A developer is expected to know the the class access model for any language they use. It's also "close enough" to the default Java type, minus the fact that we don't have packages.

Second, I'd suggest that "with sharing"/"without sharing" be used as sparingly as possible. This sounds counter-intuitive, doesn't it? After all, sharing/security is important, isn't it? Of course it is, and it also has an associated cost! Remember, the default model is "keep whatever mode you were in." Staying where you are has no cost. Entering or leaving "system mode" has a cost, even if you don't actually change modes. It's like shifting from fifth gear back into fifth gear over and over again, just to make sure you're in fifth gear. I was actually able to measure this cost, and it is small, but some function-heavy loops could have severe performance problems (16% or more CPU time).

So, "with sharing"/"without sharing" has two costs: character counts against you and CPU time to switch modes. This section should be expanded to exactly specify when "with(out) sharing" should be used. All controllers, extensions, rest service, and web services classes should use "with sharing" explicitly unless they are designed to do otherwise. All trigger classes and utility classes should use "default" sharing unless they are designed to do otherwise. No class should ever use "without sharing" unless you're explicitly trying to do something you couldn't normally do. This should be rare to non-existent.

Third, using this. I'd prefer that this was never, ever used, although the truth is, we do sometimes to because of scoping effects. Using this is the same usually the same thing as defining members to be private. It should be obvious, and if it's not, your functions are too big. They should usually fit on a screen. The only net effect of using this. is the waste of five characters per usage, which could easily become 15% of your total code base's character usage.

[ckoppelman]

I disagree with your thoughts on private, but I do see its value.

Those are an excellent points about sharing modifiers, especially the value in "stay where you are". Can you suggest a pull request?

Your argument about this., however, is not a factor of short functions. The main reason I think this is important is because Apex encourages class-level variables. I am used to keeping variables as closely scoped as possible, and in non-Salesforce controllers, I'd rarely have a class-level variable since controllers have very little (if any) state. In Salesforce, controllers do have state, and any time that state is referenced, I find it helpful when it is called out explicitly.

[brianmfear]

I'm generally on the side of "let the system do the right thing for you," so using defaults consistently seems like a good idea. This is why I generally never specify the access level if it happens to be the default. This would be less of a deal for me if we could use the public: and private: labels of old, where all elements following it had the same access level until another label changed it. While we're on the subject, I generally recommend that all declarations of the same access level should be next to each other, preferably near the top of the source.

As for this, I can see your argument for using the keyword. However, this allows us to also shadow locally scoped variables and parameters, which can introduce subtle bugs. As a further argument against, if you start using this everywhere, and you suddenly realize that variable X was really only used in one function and didn't need class scope, you now have to fix all your thises. So, my general rule is: use this to indicate you're shadowing a variable intentionally, and use this when you actually need a reference to the current class, such as passing it to a wrapper class or calling an alternate constructor. Those items aside, I don't think using or omitting this is much of a problem, as long as the rule is consistently applied either way.