5
\$\begingroup\$

I'm very new to Lua (and programming in general). I wanted to start out with a simple package that validates input based on a set of rules.

The code works as intended, but is what I have written is any good? I'd love to hear your feedback!

validator.lua

--- An input validation package written in Lua.
-- @module validator
local validator = {}

--- Local table that holds all validation functions.
-- @within rules
local rules = {}

--- Check if input holds a value.
-- @param input The input to check.
-- @return True if a input holds a value, false and error otherwise.
-- @within rules
function rules.required(input)
    if (type(input) == "string" and input:len() == 0) or
       (type(input) == "table" and not next(input))
    then
        return false, "%s has no value"
    end
    return true
end

--- Check if string or number length is greater than or equal to a given size.
-- @param input The input to check.
-- @param size The minimum size.
-- @return True if input is at least the given size, false and error otherwise.
-- @within rules
function rules.min(input, size)
    if (type(input) == "string" and input:len() < size) or
       (type(input) == "number" and input < size)
    then
        return false, "%s must be a minimum of "..size 
    end
    return true
end

--- Check if string or number length is less than or equal to a given size.
-- @param input The input to check.
-- @param size The maximum size.
-- @return True if input is at most the given size, false and error otherwise.
-- @within rules
function rules.max(input, size)
    if (type(input) == "string" and input:len() > size) or
       (type(input) == "number" and input > size)
    then
        return false, "%s must be a maximum of "..size
    end
    return true
end

--- Check if string only contains letters.
-- @param input The input to check.
-- @return True if input only contains letters, false and error otherwise.
-- @within rules
function rules.alpha(input)
    if input:find("%A") then
        return false, "%s must be alphabetic"
    end
    return true
end

--- Check if string only contains digits.
-- @param input The input to check.
-- @return True if input only contains digits, false and error otherwise.
-- @within rules
function rules.int(input)
    if input:find("%D") then
        return false, "%s must be a number"
    end
    return true
end

--- Check if string only contains letters or digits.
-- @param input The input to check.
-- @return True if input only contains letters or digits, false and error otherwise.
-- @within rules
function rules.alnum(input)
    if input:find("%W") then
        return false, "%s must be alphanumeric"
    end
    return true
end

function rules.__index()
    error("Trying to validate with non-existant rule")
end

--- Extracts all rules from a string.
-- @param[opt] rules_string The string of rules.
-- @return Table with all the rules.
-- @usage extract_rules("required|min(12)") --> {"required", "min(12)"}
local function extract_rules(rules_string)
    local rules = {}
    if rules_string then
        for rule in rules_string:gmatch("[^|]+") do
            table.insert(rules, rule)
        end
    end
    return rules
end

--- Extracts the argument from a rule.
-- @param rule The rule to extract the argument from.
-- @return The name of the rule, and the argument or nil
-- @usage extract_rule_arg("min(12)") --> "min", 12
-- @usage extract_rule_arg("required") --> "required", nil
local function extract_rule_arg(rule)
    local func_name = rule:match("%a+")
    local func_arg = rule:match("%((.+)%)")
    func_arg = tonumber(func_arg) or func_arg
    return func_name, func_arg
end

--- Validate using input and rules
-- @param inputs A table of inputs
-- @param inputs_rules A table of rules
-- @return True if validation passes, false and table with errors otherwise.
function validator.validate(inputs, inputs_rules)
    local errors = {}
    local has_errors = false

    for field, input in pairs(inputs) do
        errors[field] = {}
        local input_rules = extract_rules(inputs_rules[field])
        for _, input_rule in pairs(input_rules) do
            local rule_func_name, rule_arg = extract_rule_arg(input_rule)
            local is_valid, err = rules[rule_func_name](input, rule_arg)
            if not is_valid then
                has_errors = true
                table.insert(errors[field], err:format(field))
            end
        end
    end

    if has_errors then
        return false, errors
    end
    return true
end

return validator

Here's an example of how to use it:

local validator = require("validator")

local inputs = {
    username = "CodeReviewer",
    password = "Password123",
}

local rules = {
    username = "required|min(3)|max(20)",
    password = "required|min(8)|max(99)",
}

local is_valid, errors = validator.validate(inputs, rules)

if is_valid then
    -- Success!
else
    for field, field_errors in pairs(errors) do
        print("> "..field)
        for i=1, #field_errors do print(field_errors[i]) end
    end
end

Any input would be highly appreciated! I want to focus on becoming a better programmer in any way I can.

\$\endgroup\$
1
\$\begingroup\$

I ran the example you provided with some invalid rules. The output I receive is:

> username
> password
password must be a minimum of 12
password must be a maximum of 6

which to me appears to mean that there was some error in the username field, but was not propagated to the errors list.

Instead of creating errors[field] table for all fields, create it lazily:

if not is_valid then
    has_errors = true
    place_error(errors, field, err)
end

and the helper:

local function place_error(error_list, field, template)
    if error_list[field] == nil then
       error_list[field] = {}
    end
    --- can also write
    -- error_list[field] = error_list[field] or {}
    table.insert(error_list[field], template:format(field))
end

The function name extract_rule_arg should only extract the argument from a rule, but it returns 2 values; which is an anti-pattern imo. Rename it to for eg. parse_rule or just get_rule.


Based on current patterns parsing, you are considering |min_max_2134(12) to be the same as |min(12). It should instead raise error for the same.

local name, arg = rule:match "[^(]+%s*%((.+)%)"

and for the required and other such similar rules, the next line:

if rules[rule] then name = rule end

should be enough. Although, in this method; you'd be again discarding of another imaginary no-arg based rule: a|min(x) since rules['a'] does not exist. If you really want to raise errors for such cases, you can chose to force your rule to become the name:

name = name or rule

putting that all together:

local function parse_rule(rule)
    local name, arg = rule:match "([^(]+)%s*%((.+)%)"
    if rules[rule] then name = rule end
    name = name or rule
    arg = tonumber(arg) or arg
    return name, arg
    -- or you may choose to return the function if you rename
    -- the function as `get_rule` for eg.
    -- return rules[name], arg
end
\$\endgroup\$
1
  • \$\begingroup\$ Awesome! This is exactly what I needed. I will update my code to include your suggestions. Thank you for taking the time to review my code. \$\endgroup\$ Apr 9 '18 at 7:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.