Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tkt 405 comment3 total work load #1448

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

schch
Copy link
Contributor

@schch schch commented Aug 4, 2017

Hi there,

this adds the total load of a task (hierarchy) as described in comment 3 of issue 405 except for the part where it says work hours instead of days.

The load is calculated based on the resource assignments of a task and its subtasks similar to the total cost of a task. The value is read only and is visible at the same places as the total cost of a task.

Please let me know if you'd like any changes.

Cheers,
Christoph

@dbarashev
Copy link
Contributor

Hi Christoph,
I will look at this at the end of next week when I come back from vacation.

Copy link
Contributor

@dbarashev dbarashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Christoph,
now I have some spare time to work on GanttProject, so here is the review :) Sorry that it took so long.

@@ -78,7 +80,7 @@ public JPanel getComponent() {
CommonPanel.setupComboBoxEditor(getTable().getColumnModel().getColumn(4), myRoleManager.getEnabledRoles());

JPanel tablePanel = CommonPanel.createTableAndActions(myTable, myModel);
String layoutDef = "(ROW weight=1.0 (LEAF name=resources weight=0.5) (LEAF name=cost weight=0.5))";
String layoutDef = "(ROW weight=1.0 (LEAF name=resources weight=0.5) (COLUMN weight=0.5 (LEAF name=cost weight=0.5) (LEAF name=load weight=0.5)))";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the weights of cost and load to 0.1 -- this will group them in the upper part of the column closer to each other

private JComponent createLoadPanel() {
OptionsPageBuilder builder = new OptionsPageBuilder();

JPanel optionsPanel = new JPanel();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is either over- or under- engineered :)
GPOption and GPOptionGroup classes serve as UI-independent "properties", or as a model for UI components if you wish. OptionsPageBuilder can automatically build UI from options and groups, adds titles, makes them readonly depending on the model properties. If you use this machinery then it makes sense to utilize it completely and do not bother with UI components explicitly (and it is the recommended way, unless UI is more complex than property sheet -like)

In order to use it, remove all code at lines 130-140 and replace it with

return builder.createGroupComponent(myLoadGroup);

Also, add the option instance:

  private final DefaultDoubleOption myLoadOption = new DefaultDoubleOption("taskProperties.load.value");

and pass it to myLoadGroup in the group constructor call.

You're almost done, the remaining thing is to set the value of myLoadOption from myTask


@Override
public Load getLoad() {
return myLoad;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls replace tab with 2 spaces

*/
public class LoadAlgorithmImpl {
public Double getCalculatedLoad(Task t) {
Double total = new Double(0.0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any sense to use primitive type while calculating? Not sure if it matters these days from performance point, but summing non-primitive Double's in the loops below must heavily and pointlessly use autoboxing and unboxing


@Override
public Double getValue() {
return new LoadAlgorithmImpl().getCalculatedLoad(TaskImpl.this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, is it possible to cache the calculated result? If it is always calculated then we're going to use CPU heavily in big projects.

Probably a good place to store load, cache it and invalidate the cache is ResourceAssignmentCollectionImpl (field myAssignments) which is aware of assigned resources and their changes.

@schch
Copy link
Contributor Author

schch commented Feb 18, 2018

Hi Dmitry, getting back to it as soon as I can. Might take a couple of weeks.

@dbarashev
Copy link
Contributor

Hi, any news on this?

@schch
Copy link
Contributor Author

schch commented May 13, 2018

This will have to wait a bit longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants