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
Update value for change #71
Update value for change #71
Conversation
If you have a negative value upon dividing there is a negative value included in the change. Changing this value to `fabs()` if the division is greater than 0 will make it a positive value as long as its not negative.
Thank you for your contribution and detailed explanation. Actually, So, I don't think we need to remove the minus sign here because the value is not correct anyway. The difference number should not be shown at all in such cases. |
Corona/Model/Change.swift
Outdated
@@ -38,11 +38,11 @@ extension Change { | |||
|
|||
public var newConfirmedString: String { "+\(newConfirmed.groupingFormatted)" } | |||
public var newRecoveredString: String { currentRecovered == 0 ? "-" : "+\(newRecovered.groupingFormatted)" } | |||
public var newDeathsString: String { "+\(newDeaths.groupingFormatted)" } | |||
public var newDeathsString: String { (lastDeaths < currentDeaths) ? "+\(newDeaths.groupingFormatted)" : "\(newDeaths.groupingFormatted)" } |
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.
We just need to change the condition to the following:
newDeaths > 0 ? "+\(newDeaths.groupingFormatted)" : "-"
Corona/Model/Change.swift
Outdated
|
||
public var confirmedGrowthString: String { "↑\(confirmedGrowthPercent.kmFormatted)%" } | ||
public var recoveredGrowthString: String { currentRecovered == 0 ? "-" : "↑\(recoveredGrowthPercent.kmFormatted)%" } | ||
public var deathsGrowthString: String { "↑\(deathsGrowthPercent.kmFormatted)%" } | ||
public var deathsGrowthString: String { (deathsGrowthPercent > 0) ? "↑\(deathsGrowthPercent.kmFormatted)%" : "↓\(fabs(deathsGrowthPercent).kmFormatted)%" } |
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.
Same thing should be done here: newDeaths > 0 ? "↑\(deathsGrowthPercent.kmFormatted)%" : "-"
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.
And let's do the same for newConfirmedString
and confirmedGrowthString
Fixes the requested changes from the review.
I have went ahead and updated to reflect the request. Please let me know if i did this correctly! |
Thank you again! |
Thanks for allowing me to contribute. Being new to iOS and finding projects I understand can be difficult at times! |
If you have a negative value upon dividing there is a negative value included in the change. Changing this value to
fabs()
if the division is greater than 0 will make it a positive value by using a ternary operation.This pull request might address the issue. I am newer to iOS so if there is anything I can do to improve the PR, please let me know. The lines being addressed are 41 & 45.