• Forum
  • Lounge
  • How much attention do pay to coding styl

 
How much attention do pay to coding style?

Pages: 12
I would like to hear from you how much attention do you pay to the following:

1. Coding style
- ex. naming convention, symbol casing and similar subjective style guidelines.
2. Language specific community guidelines
- ex. cpp core guidelines such as this one: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
3. clean compile and static analysis
- ex. compiling on high warning level and dealing with warnings
- ex. static analysis warning and dealing with them
4. Commenting
- ex. commenting functions, classes, variables and namespaces
- ex. keeping your comments up to date!
5. Detailed error and exception handling
6. Code design and API design-
- this is a wide topic but doing good code design means more thinking and doing less coding.

You feel free to add additional points to this list.

Many people find some if not all of these things to be:
- time consuming
- distracting
- not pleasant to do
- not giving any value to how the program or library will function

I'm one of those who pays a lot of attention to all of these things in great detail, and can confirm that it's indeed time consuming, distracting and not giving value to functionality of a program or library.

However it's very far from not being pleasant to me, I actually enjoy being pedantic despite all the associated negatives.

I often wonder is it just me being silly or are there other people like me.
By taking a look at various open source stuff on GitHub over the years I noticed that there are very few pedantic coders.

It seems majority cares more about functionality rather than about code itself.
On another side, if you're one of those who aren't pedantic coders what are your arguments against it?
1. Coding style

I program mostly on my own nowadays. I have a naming convention I follow and I try to write the code in a consistent style (at least within the same project).

Back in school when doing projects with other people I would use whatever style we agreed upon within the group or if the course mandated a style I would use that.


2. Language specific community guidelines

I don't follow the C++ Core Guidelines or any specific guidelines at the moment. It doesn't mean I haven't looked at the C++ Core Guidelines. I am probably using many ideas that I have read there without remembering where I got it from.


3. clean compile and static analysis

I essentially use -Wall -Wextra (and -pedantic for standard conformance).

I think -Werror (turning warnings into errors) is unnecessary because if I get a warning I will always fix my code so that I don't get the warning (if I strongly disagreed with the warning I would disable it instead). When distributing the code I think it could even be harmful to use -Werror because people might use other compiler versions and get warnings that I didn't get but that usually doesn't mean anything is wrong so it's better to let the code compile.

I have tested Cppcheck once or twice but it's nothing I use on a regular basis.


4. Commenting

I write comments when I think there is something that I think need to be explained. Especially if I feel that it will take a lot of effort to understand the code later without a comment or if there is a high risk the code could easily be updated incorrectly because it was wrongly understood. Sometimes I make assumptions and if they are easily expressed in the code (e.g. as an assert) I sometimes write a comment about it.


5. Detailed error and exception handling

This varies a lot depending on the code. In a serious project I don't trust data that comes from outside. It needs to be validated. Internally inside the program I might not check everything. I'm fine with some functions having a "narrow contract" (i.e. bad input is essentially UB). Asserts helps but it's not feasible to check for everything.


6. Code design and API design

You mean like thinking through how it should work before writing it? I think I'm improving but I could do a lot better...


malibor wrote:
However it's very far from not being pleasant to me, I actually enjoy being pedantic despite all the associated negatives.

I think it's all about trade-offs. If you're doing a game on your own for fun vs. a big corporation that write software for life critical equipment then you're going to have very different requirements and make very different trade-offs.

Things like naming convention and code formatting is pretty easy to do and is not much of a burden. Trying to read inconsistent and poorly formatted code could easily become a bigger burden.


malibor wrote:
It seems majority cares more about functionality rather than about code itself.

I guess those are the people who get things done ;)
Last edited on
About the only part of coding style I really worry about is the formatting.

Visual Studio has editor settings for choosing the various format options. If I open a file that isn't "properly formatted" I use the IDE to automate the formatting.

About commenting things, I let the names do the talking for me.

Beyond that I am consistently inconsistent on doing what you talk about, malibor. A console mode program might have a different coding style than a WinAPI GUI app.
1) I try, but to be honest, a lot of crap that other people do does not stick with me because its not useful. Things like prefixes on all names, or odd line breaks (like breaking up a function header's parameter list even on 1 liners), misaligned braces (book style), and more don't make sense and I find myself forgetting them because when I am in the groove nonsense gets pushed outside my focus. If the style makes sense, I do better. As often as not, I have to do a cleanup pass after its working, though.

2) so much of this is eggheaddery that does not help anything or at best prevents some really far out one off case. As above, if it makes sense, I will do it.

3) I like to get rid of all warnings.

4) I comment a fair bit. Honestly I hold myself back on it, because many programmers get irritable about too many comments, but I want to be very clear what I did esp if its not obvious.

5) I am bad at this. To be fair, a lot of my code does not allow humans in to try to break it, that is either nonexistent (a computer is the user) or already handled (in the UI bits). I am sadly trying to catch up a bit in this area, as I do UI bits now and wow, some users will try anything.

6) I don't get a lot of luxury to design much. Its mostly fitting into existing right now, and its been forever since I had to worry much about a from scratch design.
Last edited on
Peter87 wrote:
When distributing the code I think it could even be harmful to use -Werror because people might use other compiler versions and get warnings that I didn't get ... if I strongly disagreed with the warning I would disable it instead

That's a very good reason!
I also do not turn warnings into error because often times you compile just to do quick debugging rather than building the project for release, so errors would be counterproductive for iteration and debugging.

Peter87 wrote:
You mean like thinking through how it should work before writing it? I think I'm improving but I could do a lot better...

A lot of info online can be found about "API design", there are even books written, however if you google for "code design" you won't find anything.

I've heard from some people using the term, to my knowledge code design is in fact API design that is not about API but about all the code that is not API, ex. not a library.

In your project which is ie. a console program there is no API, but this doesn't make it not a program, so anything in it is subject to design.

Ex. how are errors handled in your program?
- do you use std::cout everywhere to print errors? that's very bad code design.
- do you return literal integers as exit codes when there is failure, that's bad code design.
- do you throw exceptions of different type? that's bad code design.

Ex. how are functions and classes written?
- are there functions or classes with duplicate or similar logic? that's bad code design.
- are functions or classes contained within their own translation unit? etc.

etc..

Good code design for those sample cases is:
- you have unified error handling, ex. an enum enumerating error codes and functions which translates them to strings, and a function which returns them to system on failure.
- you have custom exception class and you always throw that, if that's not possible you further derive new exception classes from base class.
- Instead of 2 functions doing the same thing for different types you write a template instead.
- instead of 2 classes doing similar things, you write a base class for them.
this helps to reduce code bloat and improves maintenance.
- You group functions into TU's depending on purpose, and each class has it's own header\TU.

This are just examples but it's obvious that none of this is "API design", I wonder why the term "code design" is nonexistent, is there another term?

I guess those are the people who get things done ;)
Yeah, you're perhaps right, but you can get things done the other way as well, only that it's going to be slower.

We all know and experienced programs crashing right?
I think programs don't crash as often to pedantic coders ;)

So good luck to those quick coders finding a dangling pointer just because static analysis was off.
I'm sure users will understand it's a "bug" lol :)

George P wrote:
Visual Studio has editor settings for choosing the various format options. If I open a file that isn't "properly formatted" I use the IDE to automate the formatting

I'm not able to find an option to view and to auto trim trailing spaces :(
Since VS 2022 many great extensions no longer work, I currently don't have any extensions but used to have at least 10 in previous versions.

Beyond that I am consistently inconsistent on doing what you talk about, malibor. A console mode program might have a different coding style than a WinAPI GUI app


Well, when it comes to GUI it gets even more time consuming to take care of everything, so like Peter87 said, it depends whether it is about personal joy project or ex. marketing program that needs to be done ASAP.
jonnin wrote:
I try, but to be honest, a lot of crap that other people do does not stick with me because its not useful. Things like prefixes on all names, or odd line breaks (like breaking up a function header's parameter list even on 1 liners), misaligned braces (book style), and more don't make sense

Agree, a lot of crap style can be found which makes no sense, perhaps the best style is one which is very minimal without too many rules.
I think nobody likes too many style rules, there must be some freedom left.

2) so much of this is eggheaddery that does not help anything

eggheaddery lol :D

There is a lot to it, I suggest reading every day just a portion, like reading famous quotes, rather than all at once.
I'm sure everyone can find his favorite quote.
malibor wrote:
I'm not able to find an option to view and to auto trim trailing spaces :(
Since VS 2022 many great extensions no longer work, I currently don't have any extensions but used to have at least 10 in previous versions.

https://marketplace.visualstudio.com/items?itemName=Predelnik.RemoveTrailingWhitespaces-vs2022

Used with this (https://marketplace.visualstudio.com/items?itemName=MadsKristensen.TrailingWhitespaceVisualizer) to visually display trailing whitespace.

I have both installed, both work for me with VS 2022.

You might want to try just the Visualizer first, the write-up does mention it can remove trailing whitespace as well.
George P,

https://i.imgur.com/MNnufpq.png

VSIXInstaller.NoApplicableSKUsException: This extension is not installable on any currently installed products.


I don't see anything wrong with my SKU's, VS 2022 Community on Windows 10 Pro.
It's interesting though I have this error with all extensions I tried so far :/

MS gone crazy.

**EDIT:**
Btw. I figured out if I install it from extensions manager then it works!
how odd, thanks for share.
Last edited on
I have always installed extensions via the VS IDE all the way back to VS 2015. I try them out, and most I later remove, they were just too darned annoying or counterproductive. I've tried a couple of "download the extension and install it 'manually'" method and they never worked for me.

Rather a bit of a stupid and pedantic way to do things, but hey....this is MS we are talkin' about after all.

I remember the fragile nature of VS 6 way back when. The cat sneezes in the next room in his sleep and the IDE stops working.
1) Coding style. For people who work for a company, there should be specified written coding standards that all follow. If there aren't any then IMO there should be. For those that don't work for a company then obviously it's up to them as to what coding standards are used (or not). But it's useful/helpful if all code produced adheres to a similar standard.

2) Language specific community guidelines, You should be aware of C++ Core Guidelines and deviate from them only in specific documented cases.

3) Clean compile. All code should compile cleanly at highest warning level (W4 for VS). If you need to suppress a warning for a clean compile (and you're sure!), then in VS you can use #pragma warning to suppress a specific warning. eg
#pragma warning (disable : 4706) // Assignment within conditional expression

which disables the warning re assignment within a conditional expression if that is really what is wanted.

4) Comments. Write comments when there is something that needs to be explained - and don't when there isn't! One of my gripes on some code is that I can't see the code for the comments!

“Programs are meant to be read by humans and only incidentally for computers to execute.” Donald Knuth

Comments that add nothing else to the code (eg add 1 to counter) are a hindrance. A comment at the start of a code block such as 'This is an insertion sort' are useful (but don't comment every line!).

The more comments there are, the less likely they'll be updated properly when the code is changed. Put comments re a section of code starting at the left but for a specific code line put at the right of the code with the comments lined up. Some like the usage of every variable commented when defined. Fine, but only if the comment is the right of the definition.

The test of comments (or not), is that if you come back to the code in say 2 years after you write it, would you understand it if you'd forgotten all about it?

5) Errors. Never assume anything about any data that is external to the program (including from files and user input). Always but always check and validate. How this is reported is sometimes dependant upon the program/usage etc. But it should be reported.

6) Design. Programs (including classes, functions etc) should be designed as part of the program design phase before coding starts. Coding should be done from a design. It is much easier to change a design before coding starts then after coding has begun. IMO this is one area of programming that is woefully neglected in programming courses/books and that you get 'design as code' rather than 'design before code'. Design is often the hard part of programming as it's the area that requires critical, logical thinking as IMO is quite hard to teach. Teaching a programming language in terms of syntax/semantics is really quite easy. The hard part is putting the constructs together to get a working program!

7) Not mentioned but Test. Always. Test, test and test. Test each function. Test good data for correct output, check bad data is rejected/reported. use data that 'couldn't possibly arise' but will at some point if you don't test your program will handle it ok. If you ask for a name, what happens if you provide say 2 million chars? 10 million? Spaces and tabs in input? Invalid dates/times. 'Rubbish' input? Testing is an extremely important aspect and is another area usually overlooked (together with debugging).
Last edited on
malibor wrote:
do you use std::cout everywhere to print errors? that's very bad code design.

Error handling (e.g. error codes and exceptions) is not the same thing as displaying an error message to the user.

It's similar to the mistake that I often see beginners make here when they confuse "returning" (from a function) and "printing" (to the user).

malibor wrote:
It seems majority cares more about functionality rather than about code itself.
Peter87 wrote:
I guess those are the people who get things done ;)
malibor wrote:
Yeah, you're perhaps right, but you can get things done the other way as well, only that it's going to be slower.
We all know and experienced programs crashing right?
I think programs don't crash as often to pedantic coders ;)

Caring more about the functionality is not an excuse to write sloppy code. Both are important because they are related. If sloppy coding leads to bugs then that is not good functionality.

Normally the purpose of programming is to create software with the functionality that we want to have. If you write great code that doesn't do what you want then what's the point? That's why I think functionality is most important.

Last edited on
seeplus,

I basically agree with everything your said and even learned some new philosophies.

If you need to suppress a warning for a clean compile (and you're sure!), then in VS you can use #pragma warning to suppress a specific warning

There is one interesting thing about high warning level and when it comes to suppressing them, in some cases it's better to suppress warning rather than fixing them, here is one example:

/W2 5219 "implicit conversion from 'type-1' to 'type-2', possible loss of data"

Fixing this warning may bring more bugs than what it may solve even though it's level 2 warning.

Consider:
1
2
3
4
5
6
7
8
9
10
11
12
#pragma warning (default: 5219)

std::size_t f()
{
	return 0;
}

int main()
{
        int result; // I must be an int!
	result = f();
}

In some instances you are assigning to variable whose data type can't be changed, ex due to some other function expecting int, but also you can't change the design of function "f" in this case so you have 2 choices:

Choice 1, fix warning:
result = static_cast<int>(f());

Choice 2, suppress warning:
1
2
3
#pragma warning (disable: 5219)
result = f();
#pragma warning (default: 5219) 


It's very easy to be tempted by using a static_cast just to get rid of the annoying warning, but that's wrong because in the future you may indeed change either function "f" or result variable in which case you might forget about static cast resulting in unexpected data conversion and a keyword that may no longer be needed.
One good example is when you're hunting for a bug or there is a bug but everything compiles clean so how could there be a bug one may think.

Keeping track of what needs to be casted and what not becomes very hard if code changes are frequent.

Suppressing is much better because it's manageable, you can create pragmas.h and fill it with a bunch of pragma directives to suppress warnings, and include it into every TU, and then from time to time conditionally comment them out and immediately see warnings across your entire code base, no need to search or CTRL+F for anything.
If there is a bug those re-enabled warnings may give some hints, while static_cast won't give any hints, in fact it may be hiding useful information.

Another drawback of using static_cast is that it's not always quick or easy to determine whether it's correct or not, while commenting out pragmas won't do any harm.

Not mentioned but Test. Always. Test, test and test. Test each function.

Oh yeah, can't agree with that one more!
It's also not strange to hear that writing unit tests takes too much time, tests are a good fit to this category of things which everybody dislikes but are cool and beneficial.
Last edited on
Re suppress warnings. You wouldn't do it for a whole program. You'd just do it for one line - with a comment!

1
2
3
4
#pragma warning(push)
#pragma warning (disable : 4706) // Assignment within conditional expression
// Code here that triggers the warning
#pragma warning (pop) 

seeplus, that's fine with one or two warnings, but bad design if you practice it across entire code base because you'll need to resort to CTRL + F to find those pragmas when needed.
Often times suppressing annoying warnings is applicable everywhere.

Here is much better solution if you HAVE TO do it per line or function:
1
2
3
4
5
6
7
8
// macros.hpp
#define SUPPRESS(...) __pragma(warning(suppress : __VA_ARGS__))

// Source.cpp
#include macros.hpp

SUPPRESS(4706 5219)
void f() { /* suppress those 2 warnings for this block */ }


Then later if you need to toggle warning status simply redefine macro and enjoy warnings everywhere without CTRL + F, ex.:
#define SUPPRESS(...) static_cast<void>(0);
Last edited on
1
2
3
4
5
6
7
8
9
10
std::size_t f()
{
	return 0;
}

int main()
{
        int result; // I must be an int!
	result = f();
}


mixing signed and unsigned isn't a good idea. When you say that result must be an int what you probably mean is that result must be a signed type. And f() returns a type size_t as somewhere you're using .size() which returns this type.

The fix (as opposed to a kludge) is to use type ptrdiff_t. This is a signed type with the number of bytes determined whether compiling as 32 or 64 bit (like size_t) and instead of using .size() use std::ssize() which returns a type of ptrdiff_t. Voila - no compiler warnings and no future issues.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
#include <iostream>
#include <string>
using namespace std;

ptrdiff_t f() {
	return -1;
	//return ssize("foo"s);
}

int main() {
	std::ptrdiff_t result; // Must be signed!

	result = f();

	std::cout << result << '\n';
}

This is doable when for functions which you write, but difficult when using API's over which you have no control.
one function returns unsigned type and another takes the result as signed type or vice versa, problem is also same signdness.

ex.
1
2
3
4
5
6
7
8
9
10
11
12
std::string buffer; // somewhere else
// ....
std::size_t size = buffer.size();

ReadFile(
	hFile,
	&(buffer.front()),
        // Expects DWORD
	static_cast<DWORD>(size),
	&bytes_read,
	NULL
);


In this example unsigned long long is casted to unsigned long.
Is there anything we can do?

I don't think there is a solution except to throw, ex:
1
2
if (std::numeric_limits<DWORD>::max() < filesize)
    throw "overflow";


But this still doesn't let let you avoid either static_cast or using #pragma to avoid warning.
Last edited on
In that particular case, regardless of the actual size of size_t, it would be wrong to simply cast to DWORD if the buffer is supposed to be completely filled by the file contents.

1
2
3
4
5
6
7
8
9
10
11
12
13
auto data = buffer.data();
auto size = buffer.size();
while (size){
    //TODO: Replace std::min with a version that correctly
    //handles possibly heterogeneous types.
    auto bytes_to_read = (DWORD)std::min<size_t>(size, std::numeric_limits<DWORD>::max());
    DWORD bytes_read;
    auto result = ReadFile(file, data, bytes_to_read, &bytes_read, nullptr);
    //handle error & EOF

    size -= bytes_read;
    data += bytes_read;
}
helios, you haxor!

I almost wrote something wrong and then realized the portion:
1
2
size -= bytes_read;
data += bytes_read;


Very clever solution for large files.
Good thread here

1. Coding style
- ex. naming convention, symbol casing and similar subjective style guidelines.
I adopt a new style with every career change. Everywhere I've been it is everyone's responsibility to create the team's code, not Albert's code or Quentin's code or Quentin's code with Albert's patches and Zhang's edits. And yes, once I understand what the team's code is supposed to look like, that's what it will look like in every commit.

2. Language specific community guidelines
- ex. cpp core guidelines such as this one: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
Yes, please be aware of C++CG, it is the most direct window into Stroustrup's and Sutter's combined vision for what good C++ code should look like. Your vision doesn't have to match, but having justification for differences is worthwhile. As for me, as with style, specific guidelines change with teams and I adjust and perhaps influence them over time where needed, but always follow in every commit.

3. clean compile and static analysis
- ex. compiling on high warning level and dealing with warnings
- ex. static analysis warning and dealing with them

yes, plus also sanitizers. Key component of every CI pipeline.
Except -Weverything, I'm glad gcc devs keep resisting calls to copy that from clang.


4. Commenting
- ex. commenting functions, classes, variables and namespaces
- ex. keeping your comments up to date!
Variables and namespaces usually speak for themselves, but functions and classes pretty much always deserve a comment (and yes, looking for comments outdated by code changes is a part of every code review)

5. Detailed error and exception handling
always, and testing those code paths. So many people forget to test their exceptions/aborts!

6. Code design and API design-
- this is a wide topic but doing good code design means more thinking and doing less coding.
indeed a wide topic - Lakos and others wrote books about it. Fortunately it's mostly in the beginning of any product's life cycle; once it's off the ground and has users, overhauling physical design or touching every API becomes exponentially harder (unless you built in robust deprecation policies ahead of time.. few people do)
Cubbi, thank you, I'm glad to hear there are others who value coding style and design.
I didn't expect everybody would agree but at least now I know I'm not the only one or overdoing things :)

I also think this area is pretty grey, there are coding guidelines made by experts everywhere, even on MS site what is lacking is people adopting them.
I assume in the future those guidelines will be considered widely more than what is the case now.

Like one philosopher said, humanity is in it's teenagers ages, and the same could be said about programmers in regard to this topic.
Pages: 12