Ad
  • Default User Avatar

    Nicely succinct. Comments are always great for BF code.

  • Default User Avatar

    This comment is hidden because it contains spoiler information about the solution

  • Default User Avatar

    Thanks for your feedback. I'm not sure I understand the concern over using namespace std;, but I'll spend some time on the doc you linked and do some experimentation. For now I've unpublished the translation.

    You are right about the floats and Equals. I missed that detail. It'll be corrected before I republish. I'll take a look at your earlier note regarding the error messages when I have time later. Thanks!

  • Custom User Avatar
  • Custom User Avatar

    Due to specific C++ setup on Codewars, using namespace std; can be safely used only in test snippets, and not in the "complete solution", "solution setup", or in "preloaded" snippet (https://docs.codewars.com/languages/cpp/authoring#using-directives).

    Another bug in the translation: floats should not be compared for strict equality with Equals. Instead, EqualsWithDelta should be used.

  • Custom User Avatar

    I only have 1 crucial comment, and the rest are suggestions:

    1. Remove using namespace std from the solution setup + all test code. By baking it in there, it leaves the solver no choice to opt out of it. It's harmless for it be in your own solution snippet, but remove it elsewhere in the very least.
    2. do_test as it stands is just a wrapper over Assert::That without a message; it's not very useful and I suggest adding an ExtraMessage too. I think the preloaded code section is reserved for code that may be also called by the user's solution; assertions don't fall under that category since the user shouldn't interact with the testing framework. I see no reason why do_test should be in preloaded; remove it from there and put it in the submission tests instead. For fixed tests, you can either hardcode the ExtraMessage or use a stringifier like fmt::format. For random tests, use fmt::format. In summary, for random tests you can do:
       // other #includes...
       #include <fmt/ranges.h>
      
       Describe(Random_Tests)
       {
           It(Random_Test_Cases)
           {
               // ...
               do_test(blah, blah, expected);
               // ...
          }
       private:
         void do_test(const vector<int>& arr1, const vector<int>& arr2, double expected)
         {
             Assert::That(mean_square_error(arr1, arr2),
                          Equals(expected),
                          ExtraMessage(fmt::format("Incorrect output for mean_square_error({}, {}):", arr1, arr2)));
         }
       };
      
      And you can simply hardcode the strings for the sample tests.
       Describe(Fixed_Tests)
       {
           It(Example_Tests)
           {
               Assert::That(mean_square_error({ 1, 2, 3 }, { 4, 5, 6 }), Equals(9.0), ExtraMessage(R"(Incorrect output for mean_square_error({ 1, 2, 3}, { 4, 5, 6}):)"));
               // ...
          }
       };
      
    3. Add fixed test(s) for vectors of size 1 too.
    4. default_random_engine generator(chrono::system_clock::now().time_since_epoch().count()); can be changed to std::mt19937 engine{ std::random_device{} }; It's a bit easier on the eyes (with the superficial added bonus that std::mt19937 is more random).
    5. The function's name is in snake_case but everything else in the random tests is in camelCase. I suggest choosing one and sticking with it.
    6. Why do arr2.push_back(arr1.back() + randomValue());? Won't arr2.push_back(randomValue()); simply do?
  • Default User Avatar

    C++ translation ready. Please review.

  • Default User Avatar

    This comment is hidden because it contains spoiler information about the solution