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

Add dnd-character #193

Merged
merged 7 commits into from
Nov 19, 2023
Merged

Add dnd-character #193

merged 7 commits into from
Nov 19, 2023

Conversation

BNAndras
Copy link
Member

@BNAndras BNAndras commented Nov 6, 2023

I don't see a way of having students actually use randomly generated numbers, but I feel like it'd be evident in a mentoring discussion if a student didn't do that.

The test generator had trouble with the last three tests so tomorrow, I'll file another issue with the specifics.

@BNAndras
Copy link
Member Author

BNAndras commented Nov 6, 2023

Related to #172

@BNAndras
Copy link
Member Author

@kotp, should I bump the difficulty? It's at 3, but students will need to figure out how to generate random numbers. A difficulty of 4 would mark it as a medium difficulty exercise.

I'm also thinking we add a test where we create two characters and compare their total ability score across the six attributes. Then, students can't hardcode ability scores. The chance of a false positive with two totals being the same and randomly generated should be fairly low, but we can add a comment inline for students to rerun that test if needed. I'd hope you'd only need to rerun that once or twice to get it to pass.

@kotp
Copy link
Member

kotp commented Nov 14, 2023

@kotp, should I bump the difficulty? It's at 3, but students will need to figure out how to generate random numbers. A difficulty of 4 would mark it as a medium difficulty exercise.

I think making it medium would be good. We can leave it and get a feeling back via feedback, as well for the students that do the exercise.

I'm also thinking we add a test where we create two characters and compare their total ability score across the six attributes. Then, students can't hardcode ability scores. The chance of a false positive with two totals being the same and randomly generated should be fairly low, but we can add a comment inline for students to rerun that test if needed. I'd hope you'd only need to rerun that once or twice to get it to pass.

We may not need the student to run the tests more than once. If the chances of the same result are fairly low, then we run the tests that threshold, and expect that the numbers do not stay the same total. If they change even once in 4 generations for that test, then the test is probably confirming random. Also, we can use srand() to force the sequence and check in that way as well. We would know what we would expect from the next generation.

@BNAndras
Copy link
Member Author

Ooh, I didn't realize Vim had srand and rand for a while now. That'll help clean up the example.vim file as well.

@BNAndras BNAndras marked this pull request as draft November 14, 2023 19:38
@BNAndras
Copy link
Member Author

Updated the difficulty and the stub. I'll work on the test suite. The randomness is in the dice roll, not the public-facing ability and modifier functions so I'll need to figure something out or just say good enough.

Copy link
Member

@kotp kotp left a comment

Choose a reason for hiding this comment

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

Approving in case we leave the randomness checks out for now.

We might generate a few characters to ensure that the attributes change in an expected way, but this is probably something that could be driven from problem-specifications, instead.

@BNAndras
Copy link
Member Author

BNAndras commented Nov 17, 2023

Yeah, the Java track generates a reference D&D character and then up to 1000 more characters, checking each time if at least one of the character's ability scores differ from the reference character's. Therefore, it'll fail quickly if values are hardcoded. That doesn't seem like it'd take a lot of time to run.

@BNAndras BNAndras marked this pull request as ready for review November 18, 2023 19:42
@kotp kotp merged commit 1a4d475 into exercism:main Nov 19, 2023
3 checks passed
@BNAndras BNAndras deleted the dnd-exercise branch November 19, 2023 23:01
@BNAndras BNAndras added x:module/practice-exercise Work on Practice Exercises x:rep/medium Medium amount of reputation labels Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:module/practice-exercise Work on Practice Exercises x:rep/medium Medium amount of reputation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants