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.
1 Answer 1
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 field
s, 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
-
\$\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\$VirtualRaccoon– VirtualRaccoon2018年04月09日 07:20:29 +00:00Commented Apr 9, 2018 at 7:20