23

When writing some functions, I found a const keyword in parameters like this:

void MyClass::myFunction(const MyObject& obj,const string& s1,const string& s2,const string& s3){
}

often causes splitting a line into 2 lines in IDE or vim, so I want to remove all const keywords in parameters:

void MyClass::myFunction(MyObject& obj,string& s1,string& s2,string& s3){
} 

is that a valid reason to not using const? Is it maintainable to keep the parameter objects unchanged manually?

Bart van Ingen Schenau
  • 71,712
  • 20
  • 110
  • 179
ggrr
  • 5,725
  • 11
  • 35
  • 37
  • 114
    "Hey, I want to functionally change my program" to make it more readable is a bad bad reason. – Pieter B Sep 22 '16 at 08:56
  • 39
    The program is not becoming any more readable anyhow - Once you see something being handed over as `const` you have a strong hint that you don't need to bother how it might be changed in the function. – tofro Sep 22 '16 at 09:05
  • 45
    A better way to improve readability is to *reduce the number* of arguments. – 5gon12eder Sep 22 '16 at 10:36
  • 2
    a related similar issue: [putting all the stuff that's in front of the method name onto the previous line](http://programmers.stackexchange.com/q/200828/177827). I have seen this being used in C++, too. – null Sep 22 '16 at 14:15
  • @null that looks to me like a desperate attempt to not bury the function name. The name is the most important thing to spot yet many styles have us orbit the name with clutter. Any UI expert will tell you the importance of the upper left hand corner. Pitty we rarely put the name there. – candied_orange Sep 22 '16 at 20:01
  • 2
    It is a valid reason to stop your editor from auto-wrapping lines! Honestly, no matter what line length limit you use, you will always run into lines that should be just a tad longer. Such line length limits lead to all kinds of idiosyncrasies like preferring short variable names, or `typedef`-ing shorthands for commonly used types, just to save a few keystrokes here and there to keep within the line length limit. Don't do this. Format your code for the logic, not for the format of your monitor. – cmaster - reinstate monica Sep 22 '16 at 21:05
  • 1
    Readability is a reason to use `string` instead of `const string`, but using `string &` instead of `const string &` changes the semantics - it doesn't just affect readability. (I suspect it prevents you from passing a temporary `string` as an argument) – user253751 Sep 22 '16 at 21:51
  • 16
    'const' arguably *improves* the readability. This statement makes clear that obj may not change! – Charles Sep 23 '16 at 06:26
  • 4
    `in IDE or vim` Are you implying Vim is not an ide? _*runs*_ – Sebi Sep 23 '16 at 06:28
  • 14
    What would help readability is adding a space after each comma. – sam hocevar Sep 23 '16 at 07:08
  • @Charlie I do not think there is any good argument against what you are saying - const trims away an unbounded number of possible outcomes. If an object is not declared constant and is used as a function argument, who knows where that rabbit-hole leads? You may not have all the source code to know what states it might be in afterwards. If I could magically make it so, immutability would have been the default in all languages everywhere (then this question would not have been asked!) – sdenham Sep 23 '16 at 17:55
  • @immibis While declaring a C++ function argument as string rather than const string (when it could have been the latter) may not help readability from the caller's perspective, it helps readability of the function's implementation - see Charlie's comment. – sdenham Sep 23 '16 at 18:04
  • Always encapsulate the content that changes. You could also consider replacing long lists of arguments with an object, similar to .NET event arguments sent to event handlers. – code_dredd Sep 23 '16 at 22:15
  • @Charlie Your function should be simple enough that you can see whether the variable changes just by looking at the function. No need for `const` to tell you. – user253751 Sep 24 '16 at 11:28
  • 1
    Another way to increase readability is to not use param names like "obj" and "s1". I assume it's just for brevity's sake in your example, but I'd go insane if I had to work with code like that. – n_b Sep 24 '16 at 15:13
  • 1
    @immiblis If a variable is used as an argument to a function, or a method is called on it, you cannot see at a glance whether it is modified - in fact, you may not be able to tell - see Turing. – sdenham Sep 27 '16 at 13:37

6 Answers6

185

Readability is a valid reason to learn to use whitespace:

void MyClass::myFunction(
        const MyObject& obj,
        const string& s1,
        const string& s2,
        const string& s3
) {
    return;
}

Located over there the parameters won't get confused with the body of the function. By locating them on a different line you won't have to reposition them when you change the name of myFunction to something more descriptive. Not changing the parameters position when they haven't changed is something source control diff tool users will appreciate.

const means something. Don't throw it out just because you're out of space and ideas. Readability is king but breaking things in it's name is just giving up.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
  • 6
    "you won't have to reposition them when you change the name of myFunction" -- although in this case it looks as if they have been positioned to approximately line up with the opening `(`, and if that's the case then you might have to reposition them if the length of the class+function name changes by more than about 4 characters. So if you want not to have to do that, add a fixed number of levels of indentation, not a number that depends on the length of the function name. I would recommend 1 level, this answer uses 6, but any fixed number achieves the stated goal :-) – Steve Jessop Sep 22 '16 at 10:32
  • @SteveJessop A good point. Consider using what c2 calls [form 6](http://c2.com/cgi/wiki?IndentationOfParameters) – candied_orange Sep 22 '16 at 13:43
  • You can go further and introduce some tabs or other suitable whitespace to make the parameter names line up horizontally. – null Sep 22 '16 at 14:22
  • 6
    @CandiedOrange I'm surprised you didn't use "form 6" in this answer ... it's *clearly* less ugly! – svidgen Sep 22 '16 at 15:05
  • 6
    Form 6 for the win. One tabspace for one level of indentation. Simple and effective. Problem solved :) – Lightness Races in Orbit Sep 22 '16 at 15:47
  • @Null that creates vertical coupling. Now when I change the name of a parameter I have to adjust the whole block. Sure your IDE can make writing that painless but it really annoys those diff'ing in source control to see lines marked as changed that only changed their white space. – candied_orange Sep 22 '16 at 15:49
  • @LightnessRacesinOrbit: That works ok with my preferred brace style (opening brace of function block is on its own line, left-justified) but using the brace style shown in the answer, you have the parameters run together with the function body at the same indentation level with no separation. Not ok. And the answer already pointed that out. – Ben Voigt Sep 22 '16 at 16:01
  • @BenVoigt: Yes, that's one of the reasons I agreed with what CandiedOrange said that Form 6 is better than the form shown in this answer. – Lightness Races in Orbit Sep 22 '16 at 16:40
  • 2
    Ok ok form 6 it is. This is the variant that JeffGrigg mentioned on c2. – candied_orange Sep 22 '16 at 16:55
  • Up vote this comment If you'd rather see the original form6 that lines the parameters up with the body. Otherwise up vote the JeffGrigg variant comment above. – candied_orange Sep 22 '16 at 17:17
  • @CandiedOrange Why doesn't that list provide _any_ forms placing the newline between `)` and `{`? – Random832 Sep 22 '16 at 19:38
  • 4
    @random832 I think we're firmly in bike shed territory now. Here's the proper way to resolve that: search your code base for examples, consult your shops style guide, ask the developers in your shop, and if none of that yields an authoritative answer use this. Resolve disputes that could go either way with a quarter. Once the quarter has spoken be consistent! – candied_orange Sep 22 '16 at 19:50
  • I was worried this wouldn't be an answer, but am very glad to see that it's the best answer. – Dave Cousineau Sep 23 '16 at 01:49
  • One thing on each line also helps identify what happens in Version Control differentials. In the now given example a function rename would only change that line but leave the parameters untouched. You could see the calls would need to be updated elsewhere. Reciprocally if a parameter changes you can easily see it is limited to the function. – TafT Sep 23 '16 at 10:00
  • Not form6! I'm not a fan of end line comments, particularly when lined-up oh so nicely. This is one place where violating DRY is preferable to the alternatives. I use doxygen and repeat the parameter names in the multiline comment that precedes the function. – David Hammen Sep 23 '16 at 10:20
  • Related: http://programmers.stackexchange.com/questions/145055/are-there-guidelines-on-how-many-parameters-a-function-should-accept – David Hammen Sep 23 '16 at 10:20
  • I generally don't have a problem distinguishing between my parameter list and my function code. The closing parenthesis (and opening brace in this language) provide a sufficient delimiter to tell them apart. So I would say that the extra level of indentation is a matter of preference. – jpmc26 Sep 23 '16 at 17:59
  • This is a good suggestion; I think it would be better without the "learn to use whitespace" which comes off as unnecessarily condescending to me. – Don Hatch Sep 24 '16 at 01:49
52

Actually, the readability issue definitely goes the other direction. First, you can trivially solve your run-on line by the use of whitespace. But removing const doesn't just make the line shorter, it completely changes the meaning of the program.

Herb Sutter refers to the const in reference to const as the most important const because a reference to const can bind to a temporary and extend its lifetime. An lvalue reference to non-const cannot, you need a separate variable.

void foo_const(std::string const& );
void foo_nc(std::string& );

std::string some_getter();

foo_const(some_getter());      // OK
foo_const("Hello there"); // OK

foo_nc(some_getter()); // error
foo_nc("Nope");   // error

std::string x = some_getter(); // have to do this
foo_nc(x);                     // ok
std::string msg = "Really??";  // and this
foo_nc(msg);                   // ok

What this means for usability is that you'll have to introduce all these temporary variables just to be able to call your function. That's not great for readability, since these variables are meaningless and exist only because your signature is wrong.

Barry
  • 1,308
  • 8
  • 11
  • 7
    Your second paragraph gives me headaches trying to figure out if `const` is refering to `const` or to the other `const` and what `const` is indeed the most important `const` – Mindwin Remember Monica Sep 23 '16 at 17:51
  • 3
    @Mindwin I think the irony is that my second paragraph is clear and unambiguous, and I have no idea what you're talking about. – Barry Sep 23 '16 at 18:45
  • 2
    @Barry It is certainly unambiguous, but it took me a few reads of that paragraph to try to figure out what it meant. I think some of the trouble is that it can be parsed "Herb Sutter refers to the const (in reference to const) as the most important const..." or "Herb Sutter refers to const in (reference to const) as the most important const..." I believe the latter is the only valid grouping of the words, but it takes a bit to process it. Maybe there's a way to use quotation marks to remove the ambiguity? – Cort Ammon Sep 23 '16 at 18:50
  • 1
    I think some hyphenated compounds would clear this up. – shawnt00 Sep 23 '16 at 20:57
  • 2
    or specifically use `const&` rather than reference to `const` as a noun(-phrase) there – Caleth Oct 06 '16 at 08:26
22

Simple answer is "no".

The long answer is that the const keyword is part of the contract the function offers; it tells you that the argument will not be modified. The moment you remove the const that guarantee goes out of the window. Remember that you can't reasonably maintain the constness (or any other property) of something using documentation, conventions, or guidelines - if the constness is not enforced by the compiler, someone will think that they can make their work easier if they fiddle with the parameter "just a little bit". Consider:

// parameter "foo" is not modified
void fna(Foo& foo);

void fnb(const Foo& foo);

Apart from the fact that the latter version is more concise, it also provides stronger contract, and lets the compiler help you maintain your intentions. The former does nothing to prevent the fna(Foo&) function from modifying the parameter you pass to it.

As in @CandiedOrange answer, you can use whitespace to lay out the code and enhance readability.

Mael
  • 2,305
  • 1
  • 13
  • 26
14

Removing the const keyword removes readability because const communicates information to the reader and the compiler.
Reducing the horizontal length of code is good (nobody likes scrolling sideways) but there's more to const than text. You could rewrite it:

typedef string str;
typedef MyObject MObj;
void MyClass::myFunction(const MObj& o,const str& s1,const str& s2,const str& s3)

Which doesn't change the contract but fulfils the need to reduce line length. Truly I would consider the above snippet to be less readable and would opt for using more whitespace as already mentioned in CandiedOrange's answer.

const is a functionality of the code itself. You wouldn't make the function a non-member to remove the MyClass:: section of the declaration, so don't remove the const

Erdrik Ironrose
  • 4,806
  • 3
  • 13
  • 25
  • 4
    I agree that the snippet you provide is less readable. It forces a reader to go find out what `MObj` and `str` are, and certainly raises eyebrows. It's especially weird for types whose names you already have control over: E.g. Why didn't you just name `MyObject` as `MObj` to begin with? – Jason C Sep 22 '16 at 20:52
3

Is readability a valid reason to not use const in parameters?

No. Omitting const can change functionality, lose protections const provides and can potentially create less efficient code.

Is it maintainable to keep the parameter objects unchanged manually?

Rather than spend time manually formating code

void MyClass::myFunction(const MyObject& obj,const string& s1,const string& s2,const string& s3){
  //
}

Use auto formatting tools. Write the function per its functional requirements and let auto-formatting handle the presentation. Manually adjusting formatting is not as efficient as using auto formatting and using the saved time to improve other aspects of code.

void MyClass::myFunction(const MyObject& obj, const string& s1, const string& s2, 
    const string& s3) {
  // 
}
-1

As long as possible it is better to keep const visible. It improves code maintenance a lot (no guesswork to see if this method changes my arguments).

If I see lot of arguments in a method, it forces me to consider creation of project based jaron (Matrix, Employee, Rectangle, Account) that would be much shorter, easier to understand (eliminates long list of arguments to methods).

blackpen
  • 191
  • 5
  • Agreed. I was hesitant to point it out. Thus, "extreme case". – blackpen Sep 22 '16 at 21:04
  • @cmaster That said, sometimes in certain contexts, with certain programmers, it *could* stay readable. For example, in the very specific case of a program waist-deep in Windows API calls, with a reader who is used to that environment, stuff like e.g. `typedef Point * LPPOINT` and `typedef const Point * LPCPOINT`, while super obnoxious, still carries an implicit meaning, because it's consistent with immediate expectations in context. But never in the general case; it's just a weird exception I can think of. – Jason C Sep 22 '16 at 21:25
  • 1
    Yes, when I saw the question, "const unsigned long long int" came to my mind, but, it is not a good candidate to derive benefit from being either "const" or "reference". The typedef solves the general problem of shortening names; but it doesn't feel like good design to hide the very purpose of language constructs (like "const") behind it. – blackpen Sep 22 '16 at 21:37