Arik Hadas has posted comments on this change.

Change subject: frontend: reduce duplicate code related to run-once capability
......................................................................


Patch Set 9: (3 inline comments)

....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java
Line 711:         setWindow(model);
Line 712: 
Line 713:         model.init();
Line 714: 
Line 715:         // Display protocols.
This rest of the code just look long because of the temporary variables (and 
became shorter after the patch of the fluent API), basically it's all about 
lines 733-740 - I was considering whether to pass "this" to the model (it is 
needed to be set as a callback for the commands..) or not and I think that it's 
better to keep the code here and not pass the callback to the run-once model. 
the commands are somehow less related to the run-once model and more to the 
callback IMO
Line 716:         EntityModel tempVar = new EntityModel();
Line 717:         
tempVar.setTitle(ConstantsManager.getInstance().getConstants().VNCTitle());
Line 718:         tempVar.setEntity(DisplayType.vnc);
Line 719:         EntityModel vncProtocol = tempVar;


Line 739:         tempVar4.setIsCancel(true);
Line 740:         model.getCommands().add(tempVar4);
Line 741:     }
Line 742: 
Line 743:     private void OnRunOnce()
The first part (which need the selected-item) and the last part (which need 
"this" as a callback) can't be moved right? I can mode the part of the creation 
of the VM to the model, it seems right - will do
Line 744:     {
Line 745:         UserPortalItemModel selectedItem = (UserPortalItemModel) 
getSelectedItem();
Line 746:         if (selectedItem == null || selectedItem.getEntity() == null)
Line 747:         {


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
Line 1349:     }
Line 1350: 
Line 1351:     private void RunOnce()
Line 1352:     {
Line 1353:         VM vm = (VM) getSelectedItem();
Same answers :)
Line 1354:         RunOnceModel model = new WebadminRunOnceModel(vm,
Line 1355:                 
getCustomPropertiesKeysList().get(vm.getVdsGroupCompatibilityVersion()));
Line 1356:         setWindow(model);
Line 1357: 


--
To view, visit http://gerrit.ovirt.org/13207
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6561df71f093e2739e689722ead4f85428c04d06
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to