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.

Sign up for GitHub

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

Jump to bottom

Add new formatting methods to the TableBuilder and fix printing issues #405

Merged
merged 64 commits into from
Nov 24, 2022
Merged

Add new formatting methods to the TableBuilder and fix printing issues #405

merged 64 commits into from
Nov 24, 2022

Conversation

struffel
Copy link
Contributor

@struffel struffel commented Nov 19, 2022 •
edited by manoelcampos
Loading

Fix #404

This PR introduces new methods for controlling the formatting in tables built using the TableBuilderAbstract class, such as CloudletsTableBuilder. It also changes the default formatting behavior of CloudletsTableBuilder to deliver more precision by default and make potential rounding errors rarer and easier to spot.

In CloudletsTableBuilder.java it removes a rounding method that is no longer needed and refactors the code in the createTableColumns() method.

In TableBuilderAbstract.java it introduces new methods that allow users to adjust the formatting of one or multiple columns using various selectors with methods that can be chained, following the example of setTitle().

I have already prepared a new example scenario for this feature which will go in this separate PR to the cloudsimplus-example repo: cloudsimplus/cloudsimplus-examples#8

@struffel
Add setter and getter functions as well as lists
68a4ff5 
The 4 lists keep track of which column needs which formatting. The setter and getter methods will need to get expanded to reiterate through the list to update the actual formatting for every column.
@struffel
Small formatting adjustments and fixes
1ff994f
@struffel
Update set-functions to allow chaining
4837fe6
@struffel
Add TODO remark
47754ed
@struffel
Move initialization to its own function
dbd0b86
@struffel
Update default precision for time display
5dabea2
@struffel
Remove rounding function
d62cc94
@struffel
Merge remote-tracking branch 'upstream/master' into table-time-format…
0e92736 
…ting
@struffel
Use new constructor to set formatting directly.
e127845
@struffel
Move createTableColumns() into build method
5b61e8d 
This solves the issue that the variables are not initialized when createTableColumns() is executed but it does not account for additional columns being added using addColumn(). More work is required here.
@struffel
Change format handling to use column indices
3bfb9b4 
No longer uses separate lists to keep track of the formatting choices and insteads allows formatting to be changed by addressing individual column indices.
@struffel
Add functions for selecting columns by (sub)title
e20a385
@struffel
Remove newline
d3e208a
@struffel
Add JavaDocs
cbbe89b
@struffel
Trim strings to make matching easier
a1c629a
@struffel
Change spaces for layout
9f4d596
@struffel
Remove unnecessary comment
555487f
@struffel struffel mentioned this pull request Nov 19, 2022
Merged
@manoelcampos manoelcampos self-requested a review November 20, 2022 12:38
@manoelcampos
Remove redundant coments
0b89c23 
(the table title makes it clear which column we are adding)

Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Code format
63c6f2e 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Use method reference
8fbd56d 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Introduces CloudletsTableBuilder.addColumn methods for simplicity
ac15e1c 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Update TableBuilderAbstract for consistence.
6a55463 
Fix small docs issues.

Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Make parameters final in TableBuilderAbstract
b7d9b00 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Access table attribute directly inside TableBuilderAbstract
4465a59 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Place TableBuilderAbstract.addColumn methods togheter
764c83c 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Replaces getTable() to direct table access table
f9c0e64 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Renames TableBuilderAbstract.newColumn methods back to addColumn
3ce89bf 
since they are actually adding the column to the table

Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Update docs
2a9df4d 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Renames CloudletsTableBuilder.addColDataFunction to mapColDataFunction
9f1d042 
to avoid confunsion with the addColumn methods
and better semantics

Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Table column refactoring
9b8fdf9 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Start redesign
55f6bfb 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Redesign Table and TableBuilder classes
0998eab 
- Adds new AbstractTableColumn construtor that receives title, subtitle and format
  to make it easier to create columns. Accordingly add such a constructor to subclasses.
- Enables changing format for time, length, ID and PE columns
  in CloudletsTableBuilder.
  This way, the dev can define his/her own default formats upfront.
- Remove the public addColumn methods from Table interface
  and make then protected. Add newColumn methods that just create
  a column, instead of directly adding them to the table.
- When new columns are created, they are added to a temporary list
  before being inserted into the actual table.
  Columns are added to the table just after calling build.
  This way, we can add columns any time and define
  the format for such columns without worring about the order
  of these method calls (as it's expected for a builder class).

Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Copy link
Collaborator

manoelcampos commented Nov 21, 2022 •
edited
Loading

Hey @StruffelProductions

Check my last commit 0998eab.
I've made a major redesign to enable devs to change the format as they wish.
It required some work but I think it's better this way.
And now the order of method calls doesn't matter, since the table is just created when calling the build method, as a builder is supposed to do.

Could you please try it?

@manoelcampos manoelcampos reopened this Nov 21, 2022
@manoelcampos
Copy link
Collaborator

manoelcampos commented Nov 21, 2022 •
edited
Loading

Are you really using these setFormatBy... methods? I don't think they are required with this new design and may just bloat the API.

@manoelcampos manoelcampos mentioned this pull request Nov 21, 2022
Open
@manoelcampos
Fix codacy code quality issues
436062d 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Clear colsMappings before build()
790b70c 
If column definitions are changed and the build
method is called again, the list of colsMappings
should be cleared before, to add columns
again with new configurations.

Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Adds a default format constant for a cleaner code.
c17260e 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Reinstantiates colsMappings in TableBuilderAbstract instead
a035109 
of clearing to reduce computational complexity
and avoid concurrency issues.

Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos manoelcampos changed the title Add new methods for formatting to the TableBuilder / Fixes Fix #404 :: Add new methods for formatting to the TableBuilder / Fixes Nov 21, 2022
@manoelcampos manoelcampos changed the title Fix #404 :: Add new methods for formatting to the TableBuilder / Fixes Add new formatting methods to the TableBuilder and fix printing issues Nov 21, 2022
@struffel
Copy link
Contributor Author

Are you really using these setFormatBy... methods? I don't think they are required with this new design and may just bloat the API.

Hi, these setFormatBy... methods are not strictly necessary, we can remove them. The main thing that motivated me for this PR was the ability to render tables with more precise time values. The setFormatBy... methods were just a "neat" idea but the current implementation also looks good to me.

@manoelcampos
Copy link
Collaborator

Great. So I'm removing them and merging the changes. Could you please update the example PR?

@manoelcampos
Removed unused methods on TableBuilderAbstract
71ea282 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos
Remove unused import
9f6c9c0 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@struffel
Copy link
Contributor Author

Great. So I'm removing them and merging the changes. Could you please update the example PR?

Yes, I will do that later.

@manoelcampos
Remove unused methods TableColumn.matchTitle and matchSubtitle
64ed02e 
Signed-off-by: Manoel Campos <manoelcampos@gmail.com>
@manoelcampos manoelcampos merged commit 9232e8e into cloudsimplus:master Nov 24, 2022
@manoelcampos
Copy link
Collaborator

Thanks for your contributions. I've just merged them.

@struffel
Copy link
Contributor Author

I'm glad to provide something to the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manoelcampos manoelcampos manoelcampos approved these changes

Assignees

@struffel struffel

Labels
accuracy enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix accuracy issues when printing result tables and redesign TableBuilders for more flexibility
2 participants
@struffel @manoelcampos