-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat: Created physical property enum type to replace string #1149
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1149 +/- ##
============================================
+ Coverage 37.17% 37.19% +0.02%
- Complexity 5694 5708 +14
============================================
Files 713 715 +2
Lines 70704 70730 +26
Branches 8116 8122 +6
============================================
+ Hits 26287 26311 +24
- Misses 42799 42800 +1
- Partials 1618 1619 +1 ☔ View full report in Codecov by Sentry. |
2639ffd
to
12d50ed
Compare
src/main/java/neqsim/physicalproperties/PhysicalPropertyType.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest that we use more clear name for the physical properties:
DENSITY ➔ MASS_DENSITY
Reflects that it’s the density related to mass in a fluid context.
VISCOSITY ➔ DYNAMIC_VISCOSITY
Highlights that it’s the fluid’s resistance to flow.
CONDUCTIVITY ➔ THERMAL_CONDUCTIVITY
Clarifies that it’s related to heat transfer rather than electrical conductivity.
THis has been a weakness in the current code also.
12d50ed
to
553e32d
Compare
I have renamed, but not updated the tests. Should it still be possible to write initPhysicalProperties("viscosity") with the old short names? |
Yes - It would be good to keep compatibility with old code, so maybe accept both viscosity and dynamic viscosity |
6da9d03
to
856de59
Compare
77cea1b
to
fa03fdf
Compare
No description provided.