Ad
  • Custom User Avatar

    The challenge description must have changed since I submitted this, 3 months ago.

    Probably because the author realised it was a ridiculously trivial problem if you actually follow best practices and use the standard library...

  • Custom User Avatar

    Everyone who has written essay-length solutions... There's no need to reinvent the wheel!!

  • Custom User Avatar

    However, unlike other languages, ruby does not have nested methods. (http://stackoverflow.com/questions/4864191/is-it-possible-to-have-methods-inside-methods)

    What you have done here is akin to dynamically redefining try_decomp (as a regular instance method, NOT a nested method), every time you call decompose.

    So like I said, this works, but is not "the ruby way". Your code is not actually doing what you think it is doing. This would be like assuming Constants, or Threads, or Truthyness, etc works just like other languages.
    Your code may run fine, but one day you'll hit obscure bugs by doing something like this in ruby, unless you actually know what you're doing.

    (I repeat: If you really want, at least make it a proc.)

  • Custom User Avatar

    2 things I don't really like about my own solution here, and would appreciate any suggestions:

    1. My use of the @target variable. It works just fine, but is ugly!
    2. My use of the previous variable. Again, it works fine, but I can't help but feel some tiny tweak to the code would make it redundant! (Also, not being able to use named parameters without defaults, because this is running on ruby < 2.1, is a bit annoying...)
  • Custom User Avatar

    A method inside a method? :/

    It may be valid syntax, but is unnecessarily confusing! Just make it a separate, private method.
    Or if you really want, at least make it a proc.

  • Custom User Avatar

    The string interpolation is unnecessary. You can just do $1.upcase

  • Custom User Avatar

    Sorry, but this is a terrible kata.

    You are asking a bad question, teaching bad practices, and have bad tests.

    The "correct" answer to this question is simply to do something like:

    def validate(string)
      string =~ /@/
    end
    

    You should NEVER try to "fully validate" email addresses against a regex. You're not solving any real problems in doing so, and are only going to create ones! And it's also not at all important for your method to return a literal true/false (in ruby).

    There is a HUGE range of possible valid email addresses. Your test cases here a not even the tip of the iceberg. For example, the following is a perfectly valid email address: "my weird email"@domain

    This kata should be taken down.

  • Custom User Avatar

    Does not work for e.g. string = "\nabc123\n" -- this will return true; should be false.

    Also, don't need the "? true : false" part of your method. It's not vital that this method returns a boolean value at all, and even if it was, there is a simpler way to achieve this: !!string.tr("\n", "").match(/^[a-zA-Z\d]+$/)

  • Custom User Avatar
    1. You don't really need to do "!!", unless you have a specific reason to ensure this method really does return true/false (probably not!). Methods ending in "?" should have "truthy/falsey" return values, bit it's perfectly fine for these to be non-boolean. For example: Numeric#nonzero?, or File#size? do not return true/false.

    2. You should really be using \z, not \Z. \Z allows the inclusion of a newline at the end of the string.