How to reduce the likelihood of errors at the stage of writing code. Note N3

This is the third article where I want to tell you about a new pair of programming techniques that will help make the code simpler and more reliable. The previous two notes can be found here [1] and here [2]. This time the examples will be taken from the Qt project.

Introduction

Qt Project 4.7.3. I came across for studying not by chance. PVS-Studio users noticed that the analysis is somehow weak when it comes to checking projects built on the basis of the Qt library. No wonder. Static analysis allows you to find errors due to the fact that the code looks at a higher level than the compiler. Therefore, he must know certain code patterns and what the functions of the various libraries do. Otherwise, he will pass by many wonderful blunders. Let me explain with an example:

if (strcmp(My_Str_A, My_Str_A) == 0) 

It makes no sense to compare the string with itself. But the compiler is silent. He does not think about the essence of the strcmp () function. He has enough of his worries. But static analyzers may be suspicious. Qt has its own kind of string comparison function - qstrcmp (). And, accordingly, the analyzer must be trained to pay attention to the following line:

 if (qstrcmp(My_Str_A, My_Str_A) == 0) 

Mastering the Qt library and creating specialized checks is a big and systematic job. And the beginning of this work was the verification of the library itself.



Having finished viewing the warnings, I have matured a few new thoughts on improving the code, which I hope will be interesting and useful for you.

1. Process the variables in the same sequence as they are declared.

The Qt library code is very qualitative and contains almost no errors. But it revealed a large number of unnecessary initializations, unnecessary comparisons or unnecessary copying of the values ​​of variables.

')

I will give a couple of examples for clarity:

 QWidget *WidgetFactory::createWidget(...) { ... } else if (widgetName == m_strings.m_qDockWidget) { <<<=== w = new QDesignerDockWidget(parentWidget); } else if (widgetName == m_strings.m_qMenuBar) { w = new QDesignerMenuBar(parentWidget); } else if (widgetName == m_strings.m_qMenu) { w = new QDesignerMenu(parentWidget); } else if (widgetName == m_strings.m_spacer) { w = new Spacer(parentWidget); } else if (widgetName == m_strings.m_qDockWidget) { <<<=== w = new QDesignerDockWidget(parentWidget); ... } 

Here the same comparison is duplicated twice. This is not an error, but a completely redundant code. Another similar example:

 void QXmlStreamReaderPrivate::init() { tos = 0; <<<=== scanDtd = false; token = -1; token_char = 0; isEmptyElement = false; isWhitespace = true; isCDATA = false; standalone = false; tos = 0; <<<=== ... } 

Again, no mistake, but completely unnecessary double initialization of the variable. And I counted a lot of similar duplicate actions. They arise because of long lists of comparisons, assignments, initializations. It is simply not visible that the variable is already being processed, which is why extra operations appear. I can name three unpleasant consequences of having such duplicate actions:

1. Duplicates increase the length of the code. And the longer the code, the easier it is to add another duplicate.
2. If we want to change the program logic and delete one check or one assignment, then a duplicate of this action can give you several hours of fascinating debugging. Imagine yourself, you write “tos = 1” (see the first example), and then in another part of the program you are surprised that why “tos” is still zero.
3. Slow down the speed of work. Almost always they can be neglected in such situations, but it still exists.

I hope, convinced that duplicates have no place in our code. How to deal with them? As a rule, such initialization / comparisons go in a block. And there is the same block of variables. It is rational to write code so that the order of declarations of variables and the order of working with them coincide. I will give an example of not very good code:

 struct T { int x, y, z; float m; int q, w, e, r, t; } A; ... Am = 0.0; Aq = 0; Ax = 0; Ay = 0; Az = 0; Aq = 0; Aw = 0; Ar = 1; Ae = 1; At = 1; 

Naturally, this is a schematic example. The point is that when initialization is not consistent, it is much easier to write two identical strings. In the code above, the 'q' variable is initialized twice. And the error is poorly noticeable if you look at the code fluently. If you now initialize the variables in the same sequence as they are declared, then there will simply be no room for such an error. Improved version of the code:

 struct T { int x, y, z; float m; int q, w, e, r, t; } A; ... Ax = 0; Ay = 0; Az = 0; Am = 0.0; Aq = 0; Aw = 0; Ae = 1; Ar = 1; At = 1; 

Of course, I know that it is not always possible to write and work with variables in the order they are declared. But often this is possible and useful. An additional advantage will be that it will be easier for you to navigate the code.



Recommendation. When adding a new variable, try to initialize and process it in accordance with its position relative to other variables.

2. Tabular methods are good.

Tabular methods are well written by S. McConnell in the book “Perfect Code” in Chapter 18 [3]:



The table method is a schema that allows you to search for information in a table, rather than using logical expressions such as if and case. Practically everything that you can choose by means of logical operators can be selected using tables. In simple cases, logical expressions are simpler and clearer. But with the complication of logical constructions tables are becoming more attractive.



So, it’s a pity that programmers still love the huge switch () or dense forest of if-else constructions. Overcome it is very difficult. It seems that one more “case:” or a small “if” does not hurt. But it hurts. And unsuccessfully add new conditions even the most experienced programmers. As an example, a pair of defects found in Qt.

 int QCleanlooksStyle::pixelMetric(...) { int ret = -1; switch (metric) { ... case PM_SpinBoxFrameWidth: ret = 3; break; case PM_MenuBarItemSpacing: ret = 6; case PM_MenuBarHMargin: ret = 0; break; ... } 

Long-lasting switch (). And, of course, there is a forgotten operator “break”. The analyzer has detected this error due to the fact that the variable “ret” is assigned a different value twice in a row.



Perhaps, much better, it would be to get some kind of std :: map <PixelMetric, int> and clearly sign to set the correspondence between the metric and the numbers. You can come up with other table options for implementing the function.



One more example:

 QStringList ProFileEvaluator::Private::values(...) { ... else if (ver == QSysInfo::WV_NT) ret = QLatin1String("WinNT"); else if (ver == QSysInfo::WV_2000) ret = QLatin1String("Win2000"); else if (ver == QSysInfo::WV_2000) <<<=== 2003 ret = QLatin1String("Win2003"); else if (ver == QSysInfo::WV_XP) ret = QLatin1String("WinXP"); ... } 

In the code, we compare the variable 'ver' two times with the constant WV_2000. A good example is where the table method is. For example, such a method could look like this:

 struct { QSysInfo::WinVersion; m_ver; const char *m_str; } Table_WinVersionToString[] = { { WV_Me, "WinMe" }, { WV_95, "Win95" }, { WV_98, "Win98" }, { WV_NT, "WinNT" }, { WV_2000, "Win2000" }, { WV_2003, "Win2003" }, { WV_XP, "WinXP" }, { WV_VISTA,"WinVista" } }; ret = QLatin1String("Unknown"); for (size_t i = 0; i != count_of(Table_WinVersionToString); ++i) if (Table_WinVersionToString[i].m_ver == ver) ret = QLatin1String(Table_WinVersionToString[i].m_str); 

Of course, this is just a prototype, but it demonstrates well the idea of ​​tabular methods. Agree that to identify the error in such a table is much easier.



Recommendation. Do not be lazy to write a function using table methods. Yes, it will take a little time, but it will pay off later. It will be easier and faster to add new conditions, and the probability of error will be much lower.

3. Other interesting

Since Qt is a large library, despite the high quality, you can find various errors in it. The law of large numbers. The size of * .cpp, * .h and similar Qt project files is about 250 megabytes. As the error is not improbable, in the big code it is quite realistic to meet it. Based on other errors that I discovered in Qt, it is difficult to make some recommendations. Just describe some errors that I liked.

 QString decodeMSG(const MSG& msg) { ... int repCount = (lKeyData & 0xffff); // Bit 0-15 int scanCode = (lKeyData & 0xf0000) >> 16; // Bit 16-23 bool contextCode = (lKeyData && 0x20000000); // Bit 29 bool prevState = (lKeyData && 0x40000000); // Bit 30 bool transState = (lKeyData && 0x80000000); // Bit 31 ... } 

Randomly using the && operator instead of &. Notice how useful it is to have comments in the code. It immediately becomes clear that this is indeed a mistake and how bits should actually be processed.



The following example will be on the topic of long expressions:

 static ShiftResult shift(...) { ... qreal l = (orig->x1 - orig->x2)*(orig->x1 - orig->x2) + (orig->y1 - orig->y2)*(orig->y1 - orig->y1) * (orig->x3 - orig->x4)*(orig->x3 - orig->x4) + (orig->y3 - orig->y4)*(orig->y3 - orig->y4); ... } 

See a mistake? Just about, on the move and you will not notice. Well, I will. The trouble is here: "orig-> y1 - orig-> y1". I am also confused by the third multiplication, but maybe this is necessary.



Yes, another question. Do you also have computing blocks in programs? Isn't it time to try the PVS-Studio static code analyzer? So, advertised. Let's go further.



Use of uninitialized variables. They can be found in any large application:

 PassRefPtr<Structure> Structure::getterSetterTransition(Structure* structure) { ... RefPtr<Structure> transition = create( structure->storedPrototype(), structure->typeInfo()); transition->m_propertyStorageCapacity = structure->m_propertyStorageCapacity; transition->m_hasGetterSetterProperties = transition->m_hasGetterSetterProperties; transition->m_hasNonEnumerableProperties = structure->m_hasNonEnumerableProperties; transition->m_specificFunctionThrashCount = structure->m_specificFunctionThrashCount; ... } 

Here again, you need to give a hint, so as not to torment your eyes for a long time. You need to look at the initialization of the variable "transition-> m_hasGetterSetterProperties".



I am sure that almost every one of us, when I first started programming, made a mistake in the spirit:

 const char *p = ...; if (p == "12345") 

And only then came the realization of why we needed strange functions at first glance, like strcmp (). Unfortunately, C ++ is so severe that you can make such a mistake even after many years, being a professional developer with experience:

 const TCHAR* getQueryName() const; ... Query* MultiFieldQueryParser::parse(...) { ... if (q && (q->getQueryName() != _T("BooleanQuery") ... ... } 

What else to show. Here, for example, incorrectly written exchange of variable values.

 bool qt_testCollision(...) { ... t=x1; x1=x2; x2=t; t=y1; x1=y2; y2=t; ... } 

This is an example of what can be mistaken, even in very simple code. So, for the present there were no examples on the topic of going beyond the array. Will now be:

 bool equals( class1* val1, class2* val2 ) const { ... size_t size = val1->size(); ... while ( --size >= 0 ){ if ( !comp(*itr1,*itr2) ) return false; itr1++; itr2++; } ... } 

The condition "--size> = 0" is always true, since size has an unsigned type. If the same sequences are compared, then the arrays will overflow



You can continue further. I hope you, as programmers, understand that there is no possibility to describe errors in a project of such a size in one article. Therefore, the last, for a snack:

 STDMETHODIMP QEnumPins::QueryInterface(const IID &iid,void **out) { ... if (S_OK) AddRef(); return hr; } 

There should have been something in the spirit of “if (hr == S_OK)” or “if (SUCCEEDED (hr))”. The macro S_OK is nothing more than 0. Therefore, byak with an incorrect calculation of the number of links is inevitable here.

Instead of conclusion

Thanks for attention. Use static code analysis, and you can save a lot of time for more useful things than debugging and maintaining code.



Also, I will be grateful to the readers if you send me interesting examples of errors that you found in your or someone else's code and for which you can implement diagnostic rules.

Bibliographic list

1. Andrey Karpov. How to reduce the likelihood of errors at the stage of writing code. Note N1. http://habrahabr.ru/blogs/cpp/115143/
2. Andrey Karpov. How to reduce the likelihood of errors at the stage of writing code. Note N2. http://habrahabr.ru/blogs/cpp/116397/
3. McConnell S. Perfect Code. Master Class / Trans. from English - M .: Publishing and trading house "Russian Edition"; SPb .: Peter, 2005. - 896 pages: ill.