Loading collection data...
Collections are a way for you to organize kata so that you can create your own training routines. Every collection you create is public and automatically sharable with other warriors. After you have added a few kata to a collection you and others can train on the kata contained within the collection.
Get started now by creating a new collection.
Needs random tests and edge case tests. Might also be worth restructuring the tests, the current output is a bit cryptic. The Python tests provide a nice output/structure if you want to copy that.
I'm a mender so I can self approve, now that someone has looked at it I'm confident to.
I just don't like approving my own new translations without first having someone else check them.
Ah, I'm too low in Honor to be able to approve your translation. :-(
Looks good to me!
Looks good, I'll approve.
Thank you very, very much for taking the time and explaining! You were right, the tests were wonky and did not check for performance. I tried to fix those issues now. The test failure messages will read like
I'll put much more care into my next translations. :-)
Should have the inputs in the assertion messages for the users convenience so they don't need to print them. I would add it on a newline after your current assertion message, something like "\nInput: arr = $arr" and just hardcode it for the fixed tests.
Your random test generation is also wonky: There's still only one random test, and
Random.nextInt(9999)
generates numbers from 0-9998 inclusive, thereby making the maximal subarray sum always the entire array. You can useRandom.between(-9999, 10000)
, for example, to generate numbers from -9999 to 9999 inclusive. I would also personally use random array lengths. I usually make a helper function to generate individual test cases and solutions for me, then do:to run multiple tests.
I haven't tested yet, but have you checked that O(n^2) solutions pass your random test cases as they do in other languages? At the very least using Kadanes alg (O(n)) isn't enforced for Python, which is I believe the originally authored language for this kata.
Glad to have someone review my code! So, now I added newlines, grouped the basic tests and added a random test again. WDYT?
The changes so far look okay to me, now you just need a random test block now and it should be good, though with
assertResult
you can wrap all of your tests into one block like so:Add a newline to the start of the assertion messages as well, it isn't done automatically by scalatest for whatever reason, and if using the above pattern it is best practice to also add the input as well.
Glad to have someone else working on scala translations as well.
Thanks for the review and your suggestions! I tried to address most of them.
You have several issues here, the three main ones being that:
Other than that these are more style suggestions:
assertResult(<expected value>, <clue string>) {<user test result>}
is preferred over using matchers due to nicer assertion messagesArray.fill
exists, you don't need to use a for comprehension to generate your testsYou can look at some of my translations for examples if the above points are unclear, or just ask here and I'll try to explain better
Completing this Kata taught me some more Scala goodness. Yay!
Didn't know that pattern matching could help here as well. This is a great and concise solution!
Awesome, I really like how this avoids unnecessary passes over the array!