Add Two Bucket Test Generator #447
Conversation
exercises/two-bucket/TwoBucket.cs
Outdated
| @@ -9,18 +9,18 @@ public enum Bucket | |||
| public class TwoBucketResult | |||
| { | |||
| public int Moves { get; set; } | |||
| public Bucket GoalBucket { get; set; } | |||
| public int OtherBucketContents { get; set; } | |||
| public Bucket Goal_bucket { get; set; } | |||
There was a problem hiding this comment.
Please rename this to GoalBucket, to conform to standard .NET naming guidelines.
exercises/two-bucket/TwoBucket.cs
Outdated
| public Bucket GoalBucket { get; set; } | ||
| public int OtherBucketContents { get; set; } | ||
| public Bucket Goal_bucket { get; set; } | ||
| public int Other_bucket { get; set; } |
There was a problem hiding this comment.
Please rename this to OtherBucket, to conform to standard .NET naming guidelines.
exercises/two-bucket/TwoBucket.cs
Outdated
| { | ||
| public TwoBuckets(int bucketOneSize, int bucketTwoSize, Bucket startBucket) | ||
| public TwoBucket(int bucket_one, int bucket_two, Bucket start_bucket) |
There was a problem hiding this comment.
Could you rename the parameters to bucketOne, bucketTwo and startBucket?
generators/Exercises/TwoBucket.cs
Outdated
| foreach (var canonicalDataCase in canonicalData.Cases) | ||
| { | ||
| canonicalDataCase.TestedMethodType = TestedMethodType.Instance; | ||
| canonicalDataCase.SetConstructorInputParameters(new [] { "bucket_one", "bucket_two", "start_bucket"}); |
There was a problem hiding this comment.
The SetConstructorInputParameters take a param string[], which means you can simplify this to: SetConstructorInputParameters("bucket_one", "bucket_two", "start_bucket")
|
I was aware of breaking .net coding guidelines here. Wanted match canonical data properties with variables names. But I'll simply replace them in update canonical data method. Stay tuned 😉 |
|
Great! Let me say once again that I'm impressed with the quality of your PR! |
- Simplify SetConstructorInputParameters params
|
Thanks for the compliment! I think I'll grab another issue :) |
|
Yeah, this looks great @GKotfis! Just as a heads up, since you changed the name of the class, you'll also need to update the |
| } | ||
|
|
||
| public class TwoBuckets | ||
| public class TwoBucket |
There was a problem hiding this comment.
Example.cs should be changed as well.
|
Everything's green, so I'll merge it. Thanks a lot! 🎉 |
I've mixed feelings about what I've done but maybe it's ok ;)
Please feel free to give some advice in case something needs change.