m5agea
Last Updated: March 21, 2016
·
3.049K
· alexanderbrevig
Feb007acc38e70d57f7b4c205f7e8b26

Don't force your privates up my face

I think we should write code for the reader. That is, the poor colleague who must understand our intent.

I have never read one single class where the private fields or properties plays a crucial role in my understanding of said class.

Usually I visit a class for the following reasons:
- Knowing the value of a static constant, and get an overview of them
- Reading the constructor (or the setup/initialize steps) to identify preconditions the class may enforce on me (shame on you!)
- Further my understanding of how the public API works, particularly for slightly obscurely named methods and to identify side-effects
- Get an idea of the public properties/fields/getters/setters

I do not however care to read the details of private helper methods, or private constants, or fields. That's why I think you should declare your classes like this:
- Public data types, enumerations and constants
- Static methods
- Constructors & initializers
- Public methods
- All your private stuff, preferably in the order most likely to be of interest to me as a reader

Say Thanks
Respond

14 Responses
Add your response

1020
Fb974bdc0bd68fa59f433541f3b474cc

It took me a minute to understand that you were talking about the order in which to define things in a class, from top to bottom. At first I thought you were talking about documentation. Now it makes sense.

over 1 year ago ·
1035
563a59370317d8db87b2aa436e461670

A good counter argument is that when following usages, it should usually lead to interfaces, and you only read classes when you care about the implementation. In which case, having the privates at the top gives you a good clue of its structure.

over 1 year ago ·
1037
8b13e08e2eb0886384acfb8b46a90963

I disagree with a basic assumption you are making: the intended audience of the code should be other developers of the same codebase, not developers of other codebases. The use case you are describing sounds like you are using the code as an API specification. And yes, often I end up in the 'source IS the documentation' boat myself; but ideally there should be actual documentation on the API. The code should be written to be easily understood by others who will be developing the library or application, not those who simply need or want to understand what it is doing under the hood.

over 1 year ago ·
1042
Feb007acc38e70d57f7b4c205f7e8b26

@ymarcov @jesusaurus this is not supposed to be a solution to the documentation of code. I list four of my reasons for wanting to read the code, and the last point uses the word idea freely.

I start out by saying the intended reader is a colleague and not a consumer of my API. My reason for tagging this in open source is that every contributor is a colleague (at least in my opinion).

SCENARIO: I have read the API documentation, I have read high-level wiki articles but I'm in the process of fixing a bug or adding a feature. I will need to read the code before implementing.

I admit it, I'm lazy and if I can have a say I'd like you to declare fields last because I don't care about them before I'm reading a method, and when I read it I can normally deduce the type of any given variable.

over 1 year ago ·
1048
Dsc 6249

I can understand why putting the private methods / variables on the end is easier for readability but for me it doesn't affect much unless when I'm using VIM :P

over 1 year ago ·
1946
D42a7264714dee5006b9c99d2567a320

Don't force developers to manually reorder their methods/properties, it's a stupid work nobody likes to do! Besides, there will also be arguments about logical grouping of certain items. E.g. it often makes sense to place private helper methods right before public methods that use them.

//This also reminds me of enforcing CSS property order...

Just use preprocessors and documentation generators and everyone will be happy.

Bottom line: for a not-too-detailed view of your codebase there is documentation (that, btw, should be generated from code), but if you end up looking at the code, well, you're looking at the code in all its entirety. I think it's unreasonable to make assumptions about what people may or may not want to see in code.

If you feel that documentation doesn't tell you enough, improve it, make it more verbose. Or get a better doc generator tool.

P.S. Also, let's not forget that most editors/IDEs provide nice outline views & goto-anything functionality which is also helpful if you know what you want to look at.

over 1 year ago ·
1949
Feb007acc38e70d57f7b4c205f7e8b26

@dpashkevich if it's a lot of work to structure the contents of your class it's too big, that said I said nothing about the work having to be manual.
I do not agree that it makes sense to place private methods before the public that uses them. You probably made that method to abstract something away, only to ruin that abstraction by placing the code above which in my mind makes it more important (that's the whole idea behind this opinion/tip).

This is not a suggested replacement for documentation. The idea with this is to NOT force me having to looking at the code 'in all its entirety'.

Outlines tell me nothing of the intent and I very well might not know what I'm looking for. If the class is structured from what the author thinks is important from top to bottom I could safely assume that I should start reading from the top and then goto-anything from there.

over 1 year ago ·
1950
D42a7264714dee5006b9c99d2567a320

The idea with this is to NOT force me having to looking at the code 'in all its entirety'.

That's the most confusing part to me. Why do you look at the code if you don't want to look at the code? To me, you either look at the code or look at something else...

Also, do you include protected members into your statement about privates? On one hand, they're private to the outside world, on the other hand, they are used in subclasses.

over 1 year ago ·
1952
Feb007acc38e70d57f7b4c205f7e8b26

@dpashkevich it's not specified because it's hard to specify for all situations. If my protected method is protected simply because it is a common helper to inheriting types then it fits between public and private. But, if it's virtual or abstract then I could choose to put them above the public methods because I could reason that a likely reader is one that wants to make a superclass of my class. I'm not that black/white about it. Does this make sense to you? Thanks for the discussion! Do you follow any rule of thumb when you author code in regards to the textual location of the code?

over 1 year ago ·
1956
D42a7264714dee5006b9c99d2567a320

@alexanderbrevig yeah I get what you're saying. And thanks for following up, too!

First off, I code in JavaScript so the distinction between public/private/protected members is only done through doc comments (I use jsduck). But I guess that's not too important although it infers some more freedom/flexibility than in C#/Java where member visibility is enforced by the language and you have to be more strict about it.

Some time ago I tried to enforce PUBLIC/PROTECTED/PRIVATE sections in my source files and place members inside them accordingly but soon I figured it's too cumbersome and found myself not following this discipline. It takes time to scroll between different parts of the file and you might lose your thought while performing such jumps. Not to mention the extra burden if you decide to change member visibility.

Instead I learned how important documenting your code is and eventually found a good documentation generator that puts different types of class members into different sections (config parameters / runtime properties / methods / events... btw what is your approach to defining & documenting events?) and allows the reader to easily filter out private or inherited members if he wants to.

The only convention I follow is putting config variables/properties first in class definition, then the constructor, then all other methods, like they do in ExtJS. I find it more comfortable to logically group methods (this is context-aware), not by visibility. That way it's easier to follow the chain when learning how things work.

As I pointed out above, and I'll make it clearer, I don't feel there need to be different "levels of verbosity" inside the source code, the source code is already the maximum level of detail once you get there (if source code is available at all), it's all in front of you. All the "reasons" for visiting the code you listed in your original post actually look like perfect cases for visiting documentation instead. Again, if documentation doesn't give that to you (public api? preconditions? default values & constants? cmon, that all must be in the docs!), then you should really just improve the documentation, not shuffle stuff inside your source files.

over 1 year ago ·
1957
Feb007acc38e70d57f7b4c205f7e8b26

@dpashkevich These tips would not apply to JS.
However I'd argue that your convention would be the JS equivalent of this tip which is meant for type safe languages obviously. I mean, you surly have some reasoning for putting configs and contructors at the top, right?

I also want to point out that this protip is tagged open-source because that's where I think applies best. In an open source project the source is the product. Documentation is fine, but if you want to 'sell' me the projects I'll read the code, and I expect it to be easily readable. So, when I discover a new promising Java project (for instance) and every files starts with 20 lines of private member listings I leave straight off the bat.

over 1 year ago ·
1958
D42a7264714dee5006b9c99d2567a320

@alexanderbrevig you also tagged your tip with javascript tag...

Overall I agree, only I should probably clarify that when I speak about documentation I mean documentation embedded in the source code, not as a separate thing you need to maintain (besides guides/howtos maybe).

over 1 year ago ·
2014
3ec52ed58eb92026d86e62c39bdb7589

please update the tags with ruby - I found the same problem while codding ruby, never could understand why private definitions would have to be on top of the class.

over 1 year ago ·
2111
Feb007acc38e70d57f7b4c205f7e8b26

@mpapis thanks, removed javascript and added ruby.

over 1 year ago ·