Use nullptr
Needs RevisionPublic

Authored by al1xz on Jul 28 2018, 4:04 PM.

Details

Reviewers
elvisangelaccio
Group Reviewers
Dolphin

Diff Detail

Repository
R318 Dolphin
Branch
new (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1305
Build 1323: arc lint + arc unit
al1xz created this revision.Jul 28 2018, 4:04 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptJul 28 2018, 4:04 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
al1xz requested review of this revision.Jul 28 2018, 4:04 PM
al1xz updated this revision to Diff 38663.Jul 28 2018, 4:07 PM

"Fix" Typo

al1xz updated this revision to Diff 38664.Jul 28 2018, 4:09 PM

"Fix" Typo

markg added a subscriber: markg.Jul 28 2018, 5:04 PM

+1

It's good to have that!
Also note that you "can" do that in a one-liner with C++14 with a little known library feature.

delete std::exchange(object, nullptr);

But it might be too new and unknown to use throughout the code... I leave it up to you to decide.

al1xz added a comment.Jul 28 2018, 5:54 PM

+1

It's good to have that!
Also note that you "can" do that in a one-liner with C++14 with a little known library feature.

delete std::exchange(object, nullptr);

But it might be too new and unknown to use throughout the code... I leave it up to you to decide.

Thanks for your advice I appreciate it and I will keep it in mind for the future, but let's keep it "old".

hallas added a subscriber: hallas.Jul 28 2018, 6:33 PM

Just out of curiosity, what is the motivation for this change? What do we gain by assigning pointers to nullptr? Would it instead make sense to use a smart pointer? Maybe unique_ptr or a Qt version?

broulik added inline comments.
src/kitemviews/kitemlistview.cpp
308

You're re-assigning it to something else immediately after

323

Same

2744

Unrelated whitespace change

al1xz updated this revision to Diff 38763.Jul 30 2018, 11:00 AM
al1xz marked 2 inline comments as done.

Fix Typo

elvisangelaccio requested changes to this revision.Jul 31 2018, 9:12 PM
elvisangelaccio added a subscriber: elvisangelaccio.

Just out of curiosity, what is the motivation for this change? What do we gain by assigning pointers to nullptr? Would it instead make sense to use a smart pointer? Maybe unique_ptr or a Qt version?

Yes, porting to smart pointers (wherever possible) would be the best. But setting deleted pointers to null is a common practice because it will help to find some kind of bugs (even though it will hide a double delete, which isn't that good).

src/kitemviews/kitemlistview.cpp
310

Not needed, widgetCreator is a local variable that's going out of scope.

325

Same here.

2712

Same here.

src/kitemviews/kstandarditemmodel.cpp
68

Same here, but ok. This one could make sense in case one day someone removes the return.

238

Unrelated whitespace change

src/kitemviews/private/kitemlistviewanimation.cpp
189

Same here, not needed.

236

Unrelated whitespace change

src/panels/folders/folderspanel.cpp
65

Not needed.

369

Unrelated whitespace change

src/panels/places/placespanel.cpp
484

Not needed.

src/tests/kfileitemmodeltest.cpp
1645

Not needed.

src/views/dolphinview.cpp
999

Not needed.

src/views/dolphinviewactionhandler.cpp
581

Not needed.

src/views/tooltips/tooltipmanager.cpp
86

Not needed, we are already assigning it one line below.

212

Unrelated whitespace change

This revision now requires changes to proceed.Jul 31 2018, 9:12 PM