# Ruby class that validates a string

I have this class that needs to do three things:

• Determine if a string is 8 characters or longer
• Determine if a string has a number in it
• Determine if a string has a capital letter in it

NB: I don't want to use regex. Is there a better way of doing this?

class ValidateString
def initialize(string)
@string = string

if valid
puts @string
end
end

def valid
if has_number? and has_upper? and has_length?
return @string
end
end

def has_number?
@string.count("0-9") > 0
end

def has_upper?
@string =~ /[A-Z]/
end

def has_length?
@string.length >= 8
end
end

• Why not use regex? It's sounds like a valid solution for this – Flambino Apr 3 '17 at 1:23
• (And you are already using regex in has_upper? by the way) – Flambino Apr 3 '17 at 1:30
• Are you checking for password strength? Obligatory XKCD – 200_success Apr 3 '17 at 1:43

• ValidateString is strange name for a class. StringValidator would be more apt.

• Don't make your constructor print anything. Your class exists to validate a string, not print things to stdout. Leave that to some other piece of code.

• valid should be named valid? and return a boolean. And you can omit the return keyword.

• Add attr_reader :string, and replace your @string with just string

• 8 is a magic number, and should be replaced with constant (e.g. MIN_STRING_LENGTH)

• Presumably, this exists to check for simple passwords. However, you're not checking that the string contains both upper- and lowercase letters. An all-uppercase string with a number would pass validation. For passwords, you'd want to enforce that it's mixed casing.

I'd just write a method:

def valid_string?(string)
return false unless string.length >= 8
return false unless string =~ /[0-9]/
return false unless string =~ /[[:upper:]]/ # allows non-Latin letters
return false unless string =~ /[[:lower:]]/ # ditto
true
end


Of course, you'd be better off using something like zxcvbn to check password entropy. Your rules allow for the good ol' Password1 string just fine.

• "8 is a magic number, and should be replaced with constant (e.g. MIN_STRING_LENGTH)" Could you elaborate on this? Everything else makes perfect sense to me. I'm still learning the appropriate time to use constants. Why exactly should the 8 alone not be used? – user135185 Apr 3 '17 at 13:22
• @fight_dragons Because 8 by itself isn't very explanatory, whereas MIN_STRING_LENGTH makes it clear what you're doing. Granted, this isn't a terrible case, since the context makes it clear enough ("ok, string has to be at least 8 chars long, got it"), but still. If in doubt: Use a constant. Any arbitrary number you use in your code should be "labelled" by making it a constant. It keeps the code much more maintainable. – Flambino Apr 3 '17 at 13:57
• return false unless string =~ /[[:lower:]] is not asked in the specification, your function incorrectly rejects 12345689X that should be accepted. – Caridorc Apr 4 '17 at 17:40
• @Caridorc Read my answer again; the last point in particular. I'm well aware I'm making an assumption about the code's purpose. – Flambino Apr 4 '17 at 17:51
• @Flambino yup I had missed that, fair point – Caridorc Apr 4 '17 at 17:52

@Flambino 's answer can be simplified by using a simple && (and) instead of return false unless (I also removed the extra condition):

def valid_string?(string)
string.length >= 8       && \
string =~ /[0-9]/        && \
string =~ /[[:upper:]]/
end