Changes between Version 1 and Version 2 of Ticket #6447, comment 7


Ignore:
Timestamp:
2011-10-16T11:11:48+02:00 (14 years ago)
Author:
xeen

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #6447, comment 7

    v1 v2  
    11This is a duplicate of #4991. I worked on a fix for the latter, but the result is similar to olejorgenb’s patch. I believe option #4 would be the cleanest solution, but only expose MoveCommand.getLayer.
    22
    3 Option #1 has the disadvantage of changing JOSM behaviour and it’s not clear it would improve the situation (I know applications where the undo is this fine-grained. It’s annoying to have to hit undo five times before your action is actually undone. If this is really is an issue, this should be solved outside of this bug)
     3Option *1 has the disadvantage of changing JOSM behaviour and it’s not clear it would improve the situation (I know applications where the undo is this fine-grained. It’s annoying to have to hit undo five times before your action is actually undone. If this is really is an issue, this should be solved outside of this bug)
    44
    5 Option #2: I agree all nodes are likely to belong to the same data set, but we’d have to assert that. This is true as well for option #4, at least unless there’s some kind of check added that returns <null> if the nodes are on multiple layers (or data sets).
     5Option *2: I agree all nodes are likely to belong to the same data set, but we’d have to assert that. This is true as well for option #4, at least unless there’s some kind of check added that returns <null> if the nodes are on multiple layers (or data sets).
    66
    7 Option #3: sounds overcomplicated, given that all information we need is already stored elsewhere.
     7Option *3: sounds overcomplicated, given that all information we need is already stored elsewhere.
    88
    99
    1010In the current form it takes O(n²) comparisons. Only asserting all items in each collection have the same data set and that these data sets are the same would bring it down to O(2n). This is a lot better, but still feels wrong given that this is executed each mouse move while dragging. It probably doesn’t matter, because n is reasonably small.
    1111
    12 Still, I’d vote to go with #4 and enforce the same data set policy in MoveCommand’s constructor (throw an exception on error). I doubt there are any callers in JOSM that pass multi-data-set primitives to the move command, so this should be fine.
     12Still, I’d vote to go with *4 and enforce the same data set policy in MoveCommand’s constructor (throw an exception on error). I doubt there are any callers in JOSM that pass multi-data-set primitives to the move command, so this should be fine.
    1313
    1414Comments?