[haiku-bugs] Re: [Haiku] #8872: Add ability to drag drive to terminal
- From: "stippi" <trac@xxxxxxxxxxxx>
- Date: Wed, 22 Aug 2012 13:24:05 -0000
#8872: Add ability to drag drive to terminal
---------------------------------------+----------------------------
Reporter: X512 | Owner: stippi
Type: enhancement | Status: new
Priority: normal | Milestone: R1
Component: Applications/DriveSetup | Version: R1/Development
Resolution: | Keywords:
Blocked By: | Blocking:
Has a Patch: 1 | Platform: All
---------------------------------------+----------------------------
Comment (by stippi):
Patch looks fine in terms of functionality. You can improve the code a bit
in terms of simplifying it and making it more coding style compliant:
{{{
BRow* _selectedRow = RowAt(rowPoint);
if(_selectedRow) {
PartitionListRow* selectedRow
= dynamic_cast<PartitionListRow*>(_selectedRow);
}}}
You can simplify that as:
{{{
PartitionListRow* selectedRow
= dynamic_cast<PartitionListRow*>(RowAt(rowPoint));
if (selectedRow != NULL) {
}}}
It is safe to pass NULL to dynamic_cast. In addition, you also catch the
case where the returned BRow is in fact ''not'' of type PartitionListRow
in which case dynamic_cast returns NULL which the original did not check
for. BTW, I would call the variable "draggedRow" instead of "selectedRow".
Makes it clearer, IMHO.
The same change is applicable for the BStringColumn retrival. Also note
the space after "if".
What I don't like so much about the implementation is how it mixes model
specific knowledge with view specific knowledge. But it's been very long
since I looked at or even wrote the code, so it may perfectly fit with the
rest of PartitionListView.cpp, I don't remember. At the same time I am not
a fan of making things overlay complicated. But if there is an easy
alternative to your solution that better separates the model from the view
logic, then I'd prefer that. (The list view would forward the event to the
model class, if there even is such a thing, and for example let it fill
out the contents of the drag BMessage.)
Thanks for working on this!
--
Ticket URL: <http://dev.haiku-os.org/ticket/8872#comment:3>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.
Other related posts:
