Opened 5 years ago

Closed 5 years ago

#1827 closed defect (done)

Deleting the last job in a HiveJob doesn't update the details view

Reported by: abeham Owned by: abeham
Priority: medium Milestone: HeuristicLab 3.3.7
Component: Hive.Client Version: 3.3.7
Keywords: Cc:

Description

The details view still shows the job as if it was not deleted.

Change History (7)

comment:1 Changed 5 years ago by ascheibe

  • Status changed from new to accepted

comment:2 Changed 5 years ago by ascheibe

r8069 fixed bug where deleting the last job doesn't update the details view

comment:3 Changed 5 years ago by ascheibe

  • Owner changed from ascheibe to abeham
  • Status changed from accepted to reviewing

comment:4 Changed 5 years ago by abeham

  • Owner changed from abeham to ascheibe
  • Status changed from reviewing to assigned

Reviewed the change:

  • ItemTreeView<T> is a non-abstract class, provides a virtual method for removing items through a button, but no implementation of that method that actually removes the item. Since it's non-abstract it should work on its own.
  • Please consider creating a virtual method RemoveItem(T item) that you call from removeButton_Click and which you override in HiveTaskItemTreeView (make the base call at the end of that method so that the parent lookup works). Also remove the override of removeButton_Click in HiveTaskItemTreeView and move that code to the super class so that there's a working implementation of removing items.

comment:5 Changed 5 years ago by ascheibe

  • Owner changed from ascheibe to abeham
  • Status changed from assigned to reviewing

Thanks for the comments. I really don't know how to implement RemoveItem for ItemTreeView because I don't think that you can implement the removal of childs in a generic way. I have therefore made ItemTreeView abstract as well as RemoveItem. I have also added a method AddItem as the base implementation wasn't used anyway and also to be more consistent with RemoveItem. I hope this is ok, I don't know how to solve this in another way. Please see r8120 for the changes.

Last edited 5 years ago by ascheibe (previous) (diff)

comment:6 Changed 5 years ago by abeham

  • Status changed from reviewing to readytorelease

Okay, I like it better now. We can release it that way for now.

But let me make some general comments on that implementation. As it is, the ItemTreeView<T> represents the collection ItemCollection<IItemTree<T>>. The interface IItemTree<T> is required, because it has two methods among one of them is returning more IItemTree<T> child objects. That's why currently a lot of the implementation is happening in ItemTreeView<T> itself, even though as you say it cannot actually know on these things. I think IItemTree<T> is a trick to make this stuff work this way, but IMO it's not good design. If this should ever be useful for the rest of the architecture this needs to work without that interface.

We do have a couple of tree views, some of which are very sophisticated in HeuristicLab (OperatorTreeView, ScopeView, ExperimentTreeView), but there is no common implementation.

comment:7 Changed 5 years ago by mkommend

  • Resolution set to done
  • Status changed from readytorelease to closed
  • Version changed from 3.3.6 to 3.3.7
Note: See TracTickets for help on using tickets.