I'm working on a DSL for web development, similar to sinatra. The git repository is here. I've been attempting to improve this code and write it for 4 months, and as a hobby programmer, I would like to have an evaluation of general readability/maintainability, and any ways I could more logically split up my code into files. I have a test suite which I would be happy to add to the question if that is part of the norm here.
So mostly I'm looking for a code review to improve my code style, general organization (how the methods are ordered and how the files are split up), commenting practices (are my comments informative enough, are they too verbose), and general code readability. Please let me know if I should change anything to better fit the norms of this site, this is my first time here.
My goal is for the code to be readable enough that I don't have to provide a README to describe how it works, but I do have one currently if that would be helpful.
The code is for a rubygem based on rack. Here is an example usage (more examples can be found in the README):
require "atd"
request "/", "index.html" #=> for any request to / return index.html
request "/home" do
puts "/home was requested"
@http[:output] = "This is home!"
end
atd.rb
require_relative "atd/version"
require "rack"
require "webrick"
require_relative "atd/builtin_class_modifications"
require_relative "atd/routes"
# Extension packs
# require_relative "extensions/precompilers"
# The assistant technical director of your website. It does the dirty work so you can see the big picture.
module ATD
# Creates a new ATD App based on the template of {ATD::App}.
# @return [ATD::App]
# @param [Symbol] name The name of the new app and new class generated.
def new(name)
app = Class.new(App)
Object.const_set(name.to_sym, app)
app
end
# So called because each instance stores a route, and will be called if that route is reached.
# A route for the purposes of {ATD} is a parser that will be fed env in {ATD::App#call the rack app}.
class Route
attr_accessor :args, :method, :block, :path, :output, :app, :actions, :status_code
# The first two arguments must me the path and the output.
def initialize(*args, &block)
@status_code = 200
@method = [:get, :post, :put, :patch, :delete]
@method = [] if args.last.is_a?(Hash) && !(args.last[:respond_to].nil? || args.last[:ignore].nil?)
@app = :DefaultApp
parse_args(*args, &block)
end
# This works differently from a standard setter because is makes sure that a {Route} can belong to only one {App}.
def app=(app_name)
old_app = Object.const_get(@app)
new_app = Object.const_get(app_name.to_sym)
old_app.routes -= self if old_app.routes.is_a?(Array) && old_app.routes.include?(self)
new_app.routes.nil? ? new_app.routes = Array(self) : new_app.routes += Array(self)
@app = app_name
end
# @!method get(path = nil,*args)
# @param [String] path The path at which the route should receive from.
# @return ATD::Route
# Sets route to receive a get request to path and execute the block provided (if one is provided)
# @!method post(path = nil,*args)
# @param [String] path The path at which the route should receive from.
# @return ATD::Route
# Sets route to receive a post request to path and execute the block provided (if one is provided)
# @!method put(path = nil,*args)
# @param [String] path The path at which the route should receive from.
# @return ATD::Route
# Sets route to receive a put request to path and execute the block provided (if one is provided)
# @!method patch(path = nil,*args)
# @param [String] path The path at which the route should receive from.
# @return ATD::Route
# Sets route to receive a patch request to path and execute the block provided (if one is provided)
# @!method delete(path = nil,*args)
# @param [String] path The path at which the route should receive from.
# @return ATD::Route
# Sets route to receive a delete request to path and execute the block provided (if one is provided)
[:get, :post, :put, :delete, :patch].each do |method|
define_method(method) do |*args, &block|
# This conditional allows the syntax get post put "/", "Hello" because it passes
# the variables up through the different method calls.
if args.first.is_a?(ATD::Route)
@method = args.first.method
@output = args.first.output
@path = args.first.path
@args = args.first.args
@block = args.first.block
@app = args.first.app
@actions = args.first.actions
end
@method = [method] if @method.length == 5
@method += [method]
@method.uniq!
parse_args(*args, &block)
end
end
# Converts an instance of {ATD::Route} into it's Hash representation.
# The format for the Hash is listed {ATD::App#initialize here}
# @api private
def to_h
routes = {}
routes[@path] = {}
routes[@path][@method] = {}
routes[@path][@method] = {
status_code: @status_code,
output: @output,
block: @block,
args: @args,
route: self
}
routes
end
private
# This should also manage @method at some point
def parse_args(*args, &block)
args.compact!
args.flatten!
args.reject! { |arg| arg.is_a?(File) || arg.is_a?(Proc) || arg ? false : arg.empty? } # File doesn't respond to empty
@block = block
# This requires the format ATD::Route.new(path, route, args)
@path ||= args.shift
@output ||= args.shift
@args = Array(@args).concat(args) unless args.nil?
# @output should be whatever the input is unless the input is a controller/action or the input is_file_string?
if @output =~ /^\w*#\w*$/ # Check if @path is a controller#action combo
controller, action = @output.split("#")
@action = Object.const_get(controller.to_sym).method(action.to_sym)
@output = @action.call
end
# TODO: Choose one! They all work... I think...
# Method 1:
target_location = []
caller_locations.each do |caller_location|
target_dir = File.dirname(caller_location.absolute_path.to_s)
target_location.push(target_dir) unless target_dir.include?(__dir__)
end
# Method 2:
target_location = caller_locations.reject do |caller_location|
File.dirname(caller_location.absolute_path.to_s).include? __dir__
end
output_full_path = "#{File.dirname(target_location[0].absolute_path)}/assets/#{@output}"
@output = File.new(output_full_path) if File.exist?(output_full_path) && !Dir.exist?(output_full_path)
if args.is_a?(Hash) || args.last.is_a?(Hash)
@method += Array(args.last[:respond_to]) unless args.last[:respond_to].nil?
@method -= Array(args.last[:ignore]) unless args.last[:ignore].nil?
@status_code = args.last[:status] unless args.last[:status].nil?
@status_code = args.last[:status_code] unless args.last[:status_code].nil?
end
self
end
end
# A template {App} that all Apps extend. When a new App is created with {ATD.new ATD.new} it extends this class.
class App
attr_accessor :http
class << self
attr_accessor :routes # An array of instances of {ATD::Route} that belong to this {App}.
# Generates an instance of {ATD::Route}.
# Passes all arguments and the block to {Route.new the constructor} and sets the app where it was called from.
def request(*args, &block)
route = ATD::Route.new(*args, &block)
route.app = (self == Object || self == ATD::App ? :DefaultApp : name.to_sym)
route
end
alias req request
alias r request
[:get, :post, :put, :patch, :delete].each do |i|
define_method(i) do |*args, &block|
request.send(i, *args, &block) # Makes get == r.get, post == r.post, etc.
end
end
# Starts the rack server
# @param [Class] server The server that you would like to use.
# @param [Fixnum] port The port you would like the server to run on.
def start(server = WEBrick, port = 3150)
Rack::Server.start(app: new, server: server, Port: port)
end
end
# Sets up the @routes instance variable from the {.routes} class instance variable.
# Can be passed an array of instances of {ATD::Route} and they will be added to @routes.
# The format of the new @routes instance variable is:
# {"/" => {
# get: {output: "Hello World",
# block: Proc.new},
# post: {output: "Hello World",
# block: Proc.new}
# },
# "/hello" => {
# get: {output: "Hello World",
# block: Proc.new},
# post: {output: "Hello World",
# block: Proc.new
# }
# }
# }
# @param [Array] routes An array of instances of {ATD::Route}.
def initialize(routes = [])
@routes = {}
Array(routes + self.class.routes).each do |route|
route = route.clone
filename = ATD::Compilation.precompile(route, (route.args.last.is_a?(Hash) ? route.args.last[:precompile] : nil))
route_hash = route.to_h
current_route = route_hash[route.path][route.method]
current_route[:filename] = filename
block = current_route[:block]
# An instance method must be defined from the block make it the same as the controller actions. We don't want to
# convert the controller actions to blocks because if we did that, we would have to take them out of scope to allow
# them to use the @http variables.
current_route[:block] = define_singleton_method(block.object_id.to_s.tr("0-9", "a-j").to_sym, &block) unless block.nil?
current_route[:block] = route.actions unless route.actions.nil?
@routes = @routes.to_h.deep_merge(route_hash)
end
end
# Allows instance method route creation. Just another way of creating routes.
def request(*args, &block)
route = ATD::Route.new(*args, &block)
filename = ATD::Compilation.precompile(route, (route.args.last.is_a?(Hash) ? route.args.last[:precompile] : nil))
route_hash = route.to_h
route_hash[route.path][route.method][:filename] = filename
@routes = @routes.to_h.deep_merge(route_hash)
route
end
alias req request
alias r request
# Starts the rack server
# @param [Class] server The server that you would like to use.
# @param [Fixnum] port The port you would like the server to run on.
def start(server = WEBrick, port = 3150)
Rack::Server.start(app: self, server: server, Port: port)
end
# This is the method which responds to .call, as the Rack spec requires.
# It will return status code 200 and whatever output corresponds the that route if it exists, and if it doesn't
# it will return status code 404 and the message "Error 404"
def call(env)
@http = nil
route = route(env)
return error(404) if route.nil?
route[:output] = Compilation.compile(route[:filename], route[:output]) unless !route[:args].nil? && !route[:args].empty? && route[:args][0].is_a?(Hash) && route[:args][0][:compile] == false
return [route[:status_code].to_i, Hash(route[:headers]), Array(route[:output])] if route[:block].nil?
http output: route[:output], request: Rack::Request.new(env), method: env["REQUEST_METHOD"], response: Rack::Response.new(env)
return_val = method(route[:block]).call
@http[:output] = return_val if @http[:output].nil?
[@http[:status_code].to_i, Hash(@http[:headers]), Array(@http[:output])]
end
private
def route(env)
return nil if @routes[env["PATH_INFO"]].nil?
# return @routes[env["PATH_INFO"]][[]] unless @routes[env["PATH_INFO"]][[]].nil?
@routes[env["PATH_INFO"]].include_in_key?(env["REQUEST_METHOD"].downcase.to_sym)
end
def http(additional_params)
@http = { status_code: 200, headers: {} }.merge(additional_params)
end
def error(number)
[number, {}, ["Error #{number}"]]
end
end
module_function :new
end
# @return [ATD::Route]
def request(*args, &block)
ATD::App.request(args, block)
end
alias req request
alias r request
# Starts the rack server
# @param [Class] app The app you would like to start
# @param [Class] server The server that you would like to use.
# @param [Fixnum] port The port you would like the server to run on.
def start(app = DefaultApp, server = WEBrick, port = 3150)
Rack::Server.start(app: app.new, server: server, Port: port)
end
[:get, :post, :put, :patch, :delete].each do |i|
define_method(i) do |*args, &block|
request.send(i, *args, &block)
end
end
Object.const_set(:DefaultApp, Class.new(ATD::App)) # Create DefaultApp
atd/builtin_class_modifications.rb
# @!visibility private
class Hash
# Not only merges two hashes, but also merges the hashes that may be nested in.
#
# For example:
# {a: {b: "c"}}
# Is a nested hash
def deep_merge(second)
merger = proc do |_, v1, v2|
if v1.is_a?(Hash) && v2.is_a?(Hash) then v1.merge(v2, &merger)
elsif v1.is_a?(Array) && v2.is_a?(Array) then v1 | v2
elsif [:undefined, nil, :nil].include?(v2) then v1
else v2
end
end
merge(second.to_h, &merger)
end
def include_in_key?(search)
each do |key, val|
return val if key.is_a?(Array) && key.include?(search)
end
end
end
# This method only exists for the test suite, specifically {ATDTest#test_route_creation}.
# @!visibility private
class Object
# Checks if two objects are instances of the same class and that they have the same instance variables
def same_properties_as?(other_class)
other_class.class == self.class && class_instance_variables == other_class.class_instance_variables
end
# Returns the instance variables of a class
def class_instance_variables
instance_variables.map { |var| [var, instance_variable_get(var)] }.to_h
end
end
atd/routes.rb
module ATD
# This module holds everything related to the compilation of routes.
module Compilation
# A module designed to hold all the precompilation methods
module Precompiler
extend self
# Lists all filestypes that have defined precompiler methods
def filetypes
instance_methods(true) - [:filetypes]
end
end
# A module designed to hold all the compilation methods
module Compiler
extend self
# Lists all file extentions which have defined compiler methods
def filetypes
instance_methods(true) - [:filetypes]
end
end
# This method is responsible for live compilation. It takes an ATD::Route as input, and returns either
# the filename if Route.output is a file or the Route.output string if Route.output is a string.
# It will also take the file and call the corresponding compilation method on it.
def self.compile(name, contents)
return contents if name.nil?
contents = File.read(contents) if contents.is_a? File
parse(Compiler, name, contents)
end
# This method is responsible for precompilation. It takes an ATD::Route as input, and returns either
# the filename if Route.output is a file or the Route.output string if Route.output is a string.
# It will also take the file and call the corresponding precompilation method on it.
# route.output is either a full, expanded file path, a file, or a string
def self.precompile(route, *opts)
return nil if route.output.nil?
if route.output.is_a?(File)
name = route.output.is_a?(File) ? File.basename(route.output) : route.output.dup
file = route.output.is_a?(File) ? route.output.dup : File.new(route.output)
route.output = parse(Precompiler, name, File.read(file)) if opts[0].nil? || opts[0]
return name
end
route.output
end
class << self
private
def parse(type, name, contents)
name = name.split(".")
extensions = name - [name.first]
extensions.each do |extension|
if type.filetypes.include? extension.to_sym
contents = type.send(extension, contents)
extensions -= [extension]
end
end
contents
end
end
end
end
class Object
include ATD::Compilation::Compiler
end
