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

Setting individual chances in Chances has inconsistent behavior #93

Open
Samasaur1 opened this issue Dec 26, 2020 · 0 comments
Open

Setting individual chances in Chances has inconsistent behavior #93

Samasaur1 opened this issue Dec 26, 2020 · 0 comments
Labels
good first issue Good for newcomers internal change Something that only changes on the inside

Comments

@Samasaur1
Copy link
Owner

Here's the getter/setter for what I'm talking about.

public subscript(of roll: Roll) -> Chance {
get {
return chances[roll] ?? Chance.zero
}
set {
chances[roll] = newValue
}
}

As you can see, when getting a Chance, the assumption is that if it is not there, it's zero. That's what we want. However, when we set a chance to zero, we add it to the dictionary, potentially resulting in a large number of entries with Chance.zero as their values. This is unnecessary, could potentially be detrimental (storage space), and is internally inconsistent. I think it doesn't actually have an outward-facing effect, because we filter out chances that are not greater than zero:
for (roll, chance) in chances where chance.n > 0 {

but we could and probably should still change this.

@Samasaur1 Samasaur1 added good first issue Good for newcomers internal change Something that only changes on the inside labels Dec 26, 2020
@Samasaur1 Samasaur1 changed the title Setting individual chances in Changes has inconsistent behavior Setting individual chances in Chances has inconsistent behavior Dec 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers internal change Something that only changes on the inside
Projects
None yet
Development

No branches or pull requests

1 participant