Skip to content

Code cleanup and refactoring - #391

Merged
IsakNaslundBh merged 26 commits into
developfrom
LifeCycleAssessment_Toolkit-#389-CodeCleanupAndRefactoring
Oct 15, 2026
Merged

Code cleanup and refactoring#391
IsakNaslundBh merged 26 commits into
developfrom
LifeCycleAssessment_Toolkit-#389-CodeCleanupAndRefactoring

Conversation

@IsakNaslundBh

@IsakNaslundBh IsakNaslundBh commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Issues addressed by this PR

Closes #389

Cleaning up the code a bit, moving things out to hopefully make the code a bit more readable and maintainable. Main points being:

  • Imporve handling of ConstructionWasteEmssions
  • Improve handling of EvaluationConfigs (adding explicit methods for handling of the factor in there as pre-computed values)
  • Add method for getting the value of a particular Module - either itself or the constituent parts
  • Improve unit-test suite
  • Improve Descriptions

Test files

Changelog

Additional comments

@IsakNaslundBh
Add tests for resulting module values
59f155c
@IsakNaslundBh
Add method for extracting the value of a particular module, or its pa…
190a074 
…rts for the case of a combination module
@IsakNaslundBh
Add method for computing the wastefactor based on a wasterate
f258af4
@IsakNaslundBh
Refactor handling of construction waste - adding a query method to co…
06b84c6 
…mpute the value, and calling it from the modify method

Atempting to make the code a bit cleaner
@IsakNaslundBh
Refactor out the handling of the EvaluationConfigData
4f54e62 
Handling the computation of the actual values to be evaluated outside the ResultingModuleValue method.

Simplifies the code a bit, and makes it possible to re-use the existing mechanism for ensuring there are not miss matches in the modules.
@IsakNaslundBh

IsakNaslundBh commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

@BHoMBot check versioning

@bhombot-ci

bhombot-ci Bot commented Sep 4, 2026

Copy link
Copy Markdown

@IsakNaslundBh to confirm, the following actions are now queued:

  • check versioning

There are 32 requests in the queue ahead of you.

@IsakNaslundBh IsakNaslundBh self-assigned this Sep 4, 2026
@IsakNaslundBh IsakNaslundBh added the type:feature New capability or enhancement label Sep 4, 2026
@IsakNaslundBh

IsakNaslundBh commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci

bhombot-ci Bot commented Sep 18, 2026

Copy link
Copy Markdown

@IsakNaslundBh to confirm, the following actions are now queued:

  • check unit-tests

There are 3 requests in the queue ahead of you.

@IsakNaslundBh
IsakNaslundBh marked this pull request as ready for review October 13, 2026 13:29
@IsakNaslundBh
change nuget
f547fa1 
Do not need full ms compatibility - just need system drawing
@IsakNaslundBh
Update test-names to TestArgsDisplayName
75d57cd 
Keeps grouping correct, but allows for clear displaynames
@IsakNaslundBh
Minor improvement to results display
9bfefd2
@IsakNaslundBh
Change to single ResultingModuleValues method
8bbe770 
No need for IDispatch method any more, with COnfigs moved out of this space into its own method
@IsakNaslundBh
Remove not required IDictionary and dynamic casting
dc48a6d 
the IDictionary was a leftover from previous investigations
@IsakNaslundBh
Remove unused and incorrect private method
3f6418c
@IsakNaslundBh
Correct the description of SumModuleData
5ea7ab1
@IsakNaslundBh
Update descriptions for the TotalMaterialBreakdown methods
760d346
@IsakNaslundBh

IsakNaslundBh commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

@BHoMBot check compliance
@BHoMBot check required

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance
  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

There are 9 requests in the queue ahead of you.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

The check project-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

Tom-Kingstone
Tom-Kingstone reviewed Oct 14, 2026
View reviewed changes

@Tom-Kingstone Tom-Kingstone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests pass, but I have a query about ConstructionWasteEmissions.
And one spelling mistake in a property (was there before so don't know how awkward it is to change it)

Comment thread LifeCycleAssessment_Engine/Query/ConstructionWasteEmissions.cs Outdated
Comment thread LifeCycleAssessment_Engine/Modify/AddConstructionWasteEmissions.cs
@IsakNaslundBh IsakNaslundBh mentioned this pull request Oct 14, 2026
Merged
@IsakNaslundBh

IsakNaslundBh commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

@BHoMBot check compliance

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

There are 6 requests in the queue ahead of you.

@IsakNaslundBh

IsakNaslundBh commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

@BHoMBot check required

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

@IsakNaslundBh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

There are 22 requests in the queue ahead of you.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

The check project-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

@IsakNaslundBh just to let you know, I have provided a check-versioning result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @IsakNaslundBh on BHoM

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

@IsakNaslundBh just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @IsakNaslundBh on BHoM

Tom-Kingstone
Tom-Kingstone approved these changes Oct 14, 2026
View reviewed changes

@Tom-Kingstone Tom-Kingstone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comments handled, tests are passing and can't see anything else that needs changing.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

The check versioning has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

The check installer has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci

bhombot-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

@IsakNaslundBh just to let you know, I have provided a check-ready-to-merge result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @Tom-Kingstone on BHoM

@IsakNaslundBh
IsakNaslundBh merged commit 59135b6 into develop Oct 15, 2026
21 checks passed
@IsakNaslundBh
IsakNaslundBh deleted the LifeCycleAssessment_Toolkit-#389-CodeCleanupAndRefactoring branch October 15, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Tom-Kingstone Tom-Kingstone Tom-Kingstone approved these changes

Assignees

@IsakNaslundBh IsakNaslundBh

Labels

type:feature New capability or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Code cleanup from previous refactorings

2 participants

@IsakNaslundBh @Tom-Kingstone