OpenTechEngine / Discussions

The issues of this project are abused as a discussion forum for Open Tech
4 stars 0 forks source link

Improve code #16

Open Anthony-Gaudino opened 9 years ago

Anthony-Gaudino commented 9 years ago

While formatting code I saw some little things that can be done to improve the code:

1 - There's a lot of for's iterators declared in the beginning of the function, and also for's that uses post-increment instead of pre-increment. So I can change them from this:

int i;

for (i = 0; i < myvar; i++)
{
// CODE
}

To this:

for (int i = 0; i < myvar; ++i)
{
// CODE
}

which limits i scope to the for loop and increases performance by a little amount.


2 - Some code that does nothing, for example:

canContinue = false;

const saveGameDetailsList_t  &saveGameInfo
        = session->GetSaveGameManager().GetEnumeratedSavegames();

canContinue = ( saveGameInfo.Num() > 0 );

The line canContinue = false; does nothing.


Please, if you found other things like this add them in this "issue".

BielBdeLuna commented 9 years ago

interesting idea, I'll try to do the same with the script code

Anthony-Gaudino commented 9 years ago

On the for loops Instead of int one can use unsigned int, or size_t to gain even more performance and reduce chances of bugs, C++ now also has iterators (std::iterator) which gives other advantages but requires much more changes on the code.

Obviously the use of each one depends on the iterator / index use intention and how it is used inside the loop.

Anthony-Gaudino commented 9 years ago

On functions there's many variables declared but uninitialized, this is ok, but maybe some should be initialized.

Some variables also may be declared latter in code, closer to where they're going to be used.

BielBdeLuna commented 9 years ago

will this changes lower the warnings when compiling? maybe a good work would be in fixing those warnings?

Anthony-Gaudino commented 9 years ago

The changes may lower the number of warnings, but I can't be sure. I'm not working on improving the code right now, I'm just spotting some things that I can improve on next phases. On the code improvement phase I will review all code again and try to improve it, fix errors and warnings, I will look into what Coverity detected and use other tools to check the code.


On for loops, moving the index / iterator variable declaration to the for can reduce the chances of bugs because the index can only be used within the loop and it is initialized when declared. The correct use of the index type (int, size_t, etc) minimizes chances of bugs.

Removing useless code is obviously a good idea, if a code does nothing it shouldn't be there. Extra code can cause confusion and lead to bugs.

Having uninitialized variables is not bad, in fact it gives an advantage when detecting errors because static code analysis will check if somewhere in the code there is an access to a variable that was not initialized and will give a warning for it. If we initialize all variables to 0 or Null, for example, we won't have those warnings. In some cases variables should be initialized when declared, specially pointers.

Anthony-Gaudino commented 8 years ago

There are many functions that just return true or false, and some are empty.

I also found functions that receive parameters that are never used inside the function, for example, the function void idEntity::ApplyImpulse on the file Entity.cpp seems to never use the parameter idEntity *ent.

Probably those functions are virtual functions, but there's no comments saying that, I will add a FIXME so I can check this latter.