BUG:405093,404852
Details
Details
- Reviewers
dkazakov - Group Reviewers
Krita - Commits
- R37:8161893484cc: Better pointer handling in KisMirrorManager and KisMirrorAxis
Diff Detail
Diff Detail
- Repository
- R37 Krita
- Branch
- amedonosova/mirror-manager
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 9611 Build 9629: arc lint + arc unit
Comment Actions
The patch looks fine, please push it!
I understand that my inline comment might be considered as a "matter of taste", so it is not obligatory :)
| libs/ui/kis_mirror_manager.cpp | ||
|---|---|---|
| 133 | As a general rule, it is usually preferable to read the pointers from lazily calculated functions into a local variable, and only after that access them. Like that: KisMirrorAxisSP decoration = this->decoration();
if (decoration) {
decoration->->setMirrorAxisConfig(config);
}Such rule guards from weird mistakes, when someone (sometimes the author himself) inserts some code between the two calls, which affects the result of the second call and basically, invalidates the pointer. Such problems may also appear as a result of automatic merges or something like that. This comment is not a blocker of the patch. Just my opinion on the topic :) | |
Comment Actions
Thanks for the review. Your "matter of taste" comments are always a great opportunity to learn :)
As for the comment itself, on the first line of code you call the function by "this->decoration()" instead of just "decoration()". Is that just a matter of better readability, or is there a difference between the two?