Improving code efficiency by errors from cppcheck #1369
Conversation
… being used for implicit type conversions Signed-off-by: Surajjalpun2002 <[email protected]>
Surajjalpun2002
changed the title
Here the term !valid will always return false hence its useless for the logic Signed-off-by: Surajjalpun2002 <[email protected]>
avogadro/core/angleiterator.cpp
Outdated
There was a problem hiding this comment.
Also used clang-format on the 2 files.
|
Here are the build results |
|
I'll take a look at the clang-format check on Thursday. |
'd' has been allocated new memory copied the data from the 'other' object to the newly allocated memory for 'd' pointer Signed-off-by: Surajjalpun2002 <[email protected]>
'd' has been allocated new memory copied the data from the 'other' object to the newly allocated memory for 'd' pointer Signed-off-by: Surajjalpun2002 <[email protected]>
Signed-off-by: Surajjalpun2002 <[email protected]>
Such, so called 'Converting constructors', should in general be explicit for type safety reasons as that prevents unintended implicit conversions. Signed-off-by: Surajjalpun2002 <[email protected]>
avogadro/core/variant.h
Outdated
There was a problem hiding this comment.
this commit somehow has message from the previous commit, but changes in it are for the just next commit about removing unused private function
avogadro/core/array.h
Outdated
| } | ||
|
|
||
| /** Copy constructor, note the copy made of the internal data of other. */ | ||
| Array(const Array& other) : d(new Container(*other.d)) {} |
There was a problem hiding this comment.
Yeah, we don't want to do this. The Array class is copy-on-write. Please remove and/or tell cppcheck to ignore this.
Signed-off-by: Suraj Thakur <[email protected]>
Signed-off-by: Suraj Thakur <[email protected]>
…for used for implicit conversions in crystaltools.cpp Signed-off-by: Surajjalpun2002 <[email protected]>
|
Any tips? |
|
If you click "details" on any of the tests, you should be able to see the build report: (Then lots more errors) |
|
Please, let's stick to small changes. I'd rather have multiple PR with individual small changes and merge them, then have you keep pushing to this one. I don't know when you're "done" so I can review and give comments and test. (Beyond the unit tests, it's important to do functional testing because small changes can still yield big changes, e.g. in |
Signed-off-by: Surajjalpun2002 <[email protected]>
|
|
Signed-off-by: Surajjalpun2002 <[email protected]>
|
Should i close this PR as well and open a new one, as most of the changes ive made have been causing errors and i believe there is only one small commit thats reasonable in this whole pr. I hope not be anymore of a hassle for you. |
|
You're certainly not a hassle, but definitely submitting small changes as separate PR for testing would be appreciated. Understand that For example the |
Raising PR for issue #1346
Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.