Make rootgrid an unique_ptr #776
Conversation
| @@ -570,7 +568,7 @@ struct Document { | |||
| ResetFont(); | |||
| dc.SetUserScale(1, 1); | |||
| curdrawroot = WalkPath(drawpath); | |||
| int psb = curdrawroot == rootgrid ? 0 : curdrawroot->MinRelsize(); | |||
| int psb = curdrawroot == rootgrid.get() ? 0 : curdrawroot->MinRelsize(); | |||
There was a problem hiding this comment.
We need to call .get() because curdrawroot is Cell * and rootgrid is unique_ptr<Cell>.
There was a problem hiding this comment.
There was a problem hiding this comment.
I think the precedence order here is:
int psb = ((curdrawroot == rootgrid.get()) ? 0 : curdrawroot->MinRelsize());
Is this correct? If yes, I cannot compare curdrawroot and rootgrid directly?
| Cell *c = *r->grid->cells; | ||
| FillXML(c, doc.GetRoot(), k == A_IMPXMLA); | ||
| if (!c->HasText() && c->grid) { | ||
| *r->grid->cells = nullptr; | ||
| delete r; | ||
| r = c; | ||
| r.reset(c); |
There was a problem hiding this comment.
Here I manipulate the pointer r directly, so I need a reference to the pointer. r is (by reference) the same as rootgrid of the newly created Document created by line 467. I cannot see how I can borrow the pointer here. But maybe you can help me out here? With .reset(T*) I instruct the owner directly to delete the previous managed resource.
| @@ -487,7 +486,7 @@ struct System { | |||
|
|
|||
| if (as.size()) switch (k) { | |||
| case A_IMPTXTI: { | |||
| Cell *r = InitDB(1); | |||
| Cell *r = InitDB(1).get(); | |||
There was a problem hiding this comment.
Here I can borrow the pointer r because only the substructure r->grid is manipulated.
| @@ -608,7 +608,7 @@ struct Grid { | |||
| const int root_grid_spacing = 2; // Can't be adjusted in editor, so use a default. | |||
| const int font_size = 14 - indent / 2; | |||
| const int grid_border_width = | |||
| cell == doc->rootgrid ? root_grid_spacing : user_grid_outer_spacing - 1; | |||
| cell == doc->rootgrid.get() ? root_grid_spacing : user_grid_outer_spacing - 1; | |||
There was a problem hiding this comment.
See my other comment about using it as boolean
There was a problem hiding this comment.
Also here: I think the precedence order is
const int grid_border_width = (((cell == doc->rootgrid.get()) ? root_grid_spacing : user_grid_outer_spacing) - 1)
There was a problem hiding this comment.
This is not a unary operation or am I missing something?
There was a problem hiding this comment.
Yes, weird, they have overloads to directly compare against null, but not raw pointers https://en.cppreference.com/w/cpp/memory/unique_ptr/operator_cmp
There was a problem hiding this comment.
Unfortunately they have not, would make things easier...
|
Thanks for the merge! |
This is the second approach. I used the wrong comment type (review comment), so maybe this more cleaner now.