Ad
  • Custom User Avatar

    You're correct, and I would certainly use a Builder even on short strings.

    I wrote this solution when I was brand new to Go and had to pick it up quickly while moving teams at work. I don't think my solution is best practice despite the 12 votes :D

  • Custom User Avatar
  • Custom User Avatar

    Once a translation/fork is approved and merged, wouldn't any future maintainence that needs to be done require a new fork from the base?

    I am still very green to codewars, but it seems like anyone could pick up where the previous fork author left off after it is merged?

    Regardless, thanks for taking the time to review the fork.

  • Default User Avatar

    Sorry but I don't approve translations by newcomers : too often they soon leave CW and doing so they can't maintain their work.

  • Custom User Avatar

    I created a fork with a clearer description: Here.

    I noticed several concerns that were voiced in the discourse for this kata:

    1. There were multiple threads in the discourse concerning the divisors of sqaures. For example the number 1 has the divisors [1], but some were claiming it should be [1, 1] (which is false).
    2. This wording "... that the sum of their squared divisors is itself a square." is mildly crowded and was occaisonally confused for "... that the sum of their square divisors is itself a square."
    3. The wording regarding the return value of the function was very crowded and somewhat unclear.

    To address these concerns:

    • I added a description of what the set of divisors is, including the fact that the divisors in the list must be unique (non-repeating).
    • I reframed the example to more clearly represent the sequence of operations that are needed.
    • I cleaned up the explanation of the return value to more clearly show the distinction between languages.
  • Custom User Avatar

    Changed the description to better describe what the set of divisors is.
    I noticed many people in the discourse complaining that 1's divisors are [1, 1].

    That just isn't the case, so I thought maybe a description of divisors would help.

  • Custom User Avatar

    Golang Translation with Improved Testing

    There are a few differences that I want to highlight:

    • Test case added for minimum n (n = 1)
    • Test case added for maximum n (n = 10000)
    • Test cases added for randomly generated n (1 ≤ n ≤ 10000)
    • Solution changed to be capable of solving high n (n ≈ 10000)

    The previous solution couldn't handle n = 10,000. It would panic with a runtime error: index out of range. The reason for the panic is that the sieve was of size 70,000 which isn't enough natural numbers to find 10,000 ludic numbers. If you attempt to increase the size of the sieve, then the process will timeout. My solution can handle n ≈ 25000 before timeout occurs.

    The modifications to the test cases are to increase test coverage to include the minimum and maximum n

  • Custom User Avatar
  • Custom User Avatar

    Based on the description: ... a "bad" control string is produced e.g. aaaxbbbbyyhwawiwjjjwwm with letters not from a to m.

    Then a test case with input "aaabbbAAABBBcccCCC" should expect "9/18", and fits with the existing description.

    Maybe the description should specify ... with lowercase letters not from a to m.?

    Otherwise, a truly valid soltion should be able to handle uppercase letters being in the control string as well.

  • Custom User Avatar

    I like what you've done using the logical or!

    But, I do have a suggestion as well.

    If you replace if i == 0 || n < min with if n < min || i == 0 you may get an extremely miniscule performance increase due to golang's short circuit evaluation.

    Since i == 0 is more often false, then each if statement will have to evaluate 2 expressions instead of just 1 (in the common case).

    The performace gain is very minimal, but I thought I would point it out anyways.

  • Custom User Avatar

    I know this is a very ... very old post, but I thought I would point out that the issue is that your solution string has an extra space character at the end.

    Basically "AbC DeF " != "AbC DeF", which is the correct behaivor for the test case shown.

  • Custom User Avatar

    For clarity I suggest replacing str[i:i+1] with str[i].

    My only other suggestion would be to allocate memory for result before entering the loop. Since the length of result is known (len(result) == len(str)), you save time allocating it in the beginning vs. reallocation with each string concatenation (result += ...).

  • Custom User Avatar

    Based on my understanding of string concatenation, for long strings your function may have performance loss.
    The reason would be that additional memory allocations will be made with every string concatentation.

    You may find using a strings.Builder would help with performance (on long strings) due to less memory allocations being made.

  • Custom User Avatar

    There are a few differences that I want to highlight:

    • Test case added for minimum n (n = 1)
    • Test case added for maximum n (n = 10000)
    • Test cases added for randomly generated n (1 ≤ n ≤ 10000)
    • Solution changed to be capable of solving high n (n ≈ 10000)

    The previous solution couldn't handle n = 10,000. It would panic with a runtime error: index out of range.
    The reason for the panic is that the sieve was of size 70,000 which isn't enough natural numbers to find
    10,000 ludic numbers. If you attempt to increase the size of the sieve, then the process will timeout.
    My solution can handle n ≈ 25000 before timeout occurs.

    The modifications to the test cases are to increase test coverage to include the minimum and maximum n.