From 9003075ec7cdd485eb793eb741f9ba2794c697b6 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Thu, 14 Apr 2016 20:46:12 -0300 Subject: [PATCH] Introduce types for forward compiler compatiblity After Crystal 0.15, compiler will require declare the types used by instance variables on classes. This require changes to the usage of `Radix::Tree` by introducing the type of payload elements it will handle: # Will only support symbols as payload tree = Radix::Tree(Symbol).new tree.add "/", :root # Error: cannot add node with anything other than Symbol tree.add "/meaning-of-life", 42 The changes ensure future compatibility with Crystal and also enforces a more declarative usage of `Radix::Tree`. If necessary, you can combine multiple types to ensure a tree can contain all the wide range of payloads you need: tree = Radix::Tree.new(Foo | Bar | Symbol).new tree.add "/", :root tree.add "/foo", foo_instance This change includes: - Tree, Node and Result has been updated to require types. - Node is capable of have optional payload (from defined type). - Documentation has been updated to reflect this change. --- CHANGELOG.md | 9 ++++ README.md | 38 ++++++++++++++--- spec/radix/node_spec.cr | 40 ++++++++++-------- spec/radix/result_spec.cr | 26 ++++++------ spec/radix/tree_spec.cr | 86 +++++++++++++++++++++++++-------------- src/radix/node.cr | 54 +++++++++++++++--------- src/radix/result.cr | 26 ++++++------ src/radix/tree.cr | 20 ++++----- 8 files changed, 190 insertions(+), 109 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 361a2b8..a10fb8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ All notable changes to Radix project will be documented in this file. This project aims to comply with [Semantic Versioning](http://semver.org/), so please check *Changed* and *Removed* notes before upgrading. +## [Unreleased] +### Fixed +- Improve forward compatibility with newer versions of the compiler by adding + missing types to solve type inference errors. + +### Changed +- `Radix::Tree` now requires the usage of a type which will be used as node's + payload. See [README](README.md) for details. + ## [0.2.1] - 2016-03-15 ### Fixed - Correct `Result#key` incorrect inferred type. diff --git a/README.md b/README.md index d83a282..1cd80bf 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,11 @@ # Radix Tree -[![Build Status](https://travis-ci.org/luislavena/radix.svg?branch=master)](https://travis-ci.org/luislavena/radix) -[![docrystal.org](http://docrystal.org/badge.svg?style=round)](http://docrystal.org/github.com/luislavena/radix) - [Radix tree](https://en.wikipedia.org/wiki/Radix_tree) implementation for Crystal language +[![Build Status](https://travis-ci.org/luislavena/radix.svg?branch=master)](https://travis-ci.org/luislavena/radix) +[![docrystal.org](http://docrystal.org/badge.svg?style=round)](http://docrystal.org/github.com/luislavena/radix) + ## Installation Add this to your application's `shard.yml`: @@ -18,12 +18,14 @@ dependencies: ## Usage +### Building Trees + You can associate a *payload* with each path added to the tree: ```crystal require "radix" -tree = Radix::Tree.new +tree = Radix::Tree(Symbol).new tree.add "/products", :products tree.add "/products/featured", :featured @@ -34,6 +36,30 @@ if result.found? end ``` +The types allowed for payload are defined on Tree definition: + +```crystal +tree = Radix::Tree(Symbol).new + +# Good, since Symbol is allowed as payload +tree.add "/", :root + +# Compilation error, Int32 is not allowed +tree.add "/meaning-of-life", 42 +``` + +Can combine multiple types if needed: + +```crystal +tree = Radix::Tree(Int32 | String | Symbol).new + +tree.add "/", :root +tree.add "/meaning-of-life", 42 +tree.add "/hello", "world" +``` + +### Lookup and placeholders + You can also extract values from placeholders (as named segments or globbing): ```crystal @@ -53,8 +79,8 @@ Please see `Radix::Tree#add` documentation for more usage examples. Pretty much all Radix implementations have their limitations and this project is no exception. -When designing and adding *paths* to build a Tree, please consider that two -different named parameters cannot share the same level: +When designing and adding *paths* to a Tree, please consider that two different +named parameters cannot share the same level: ```crystal tree.add "/", :root diff --git a/spec/radix/node_spec.cr b/spec/radix/node_spec.cr index 2578f68..b1c4028 100644 --- a/spec/radix/node_spec.cr +++ b/spec/radix/node_spec.cr @@ -4,7 +4,7 @@ module Radix describe Node do describe "#key=" do it "accepts change of key after initialization" do - node = Node.new("abc") + node = Node(Nil).new("abc") node.key.should eq("abc") node.key = "xyz" @@ -23,39 +23,45 @@ module Radix node.payload.should eq(1_000) end + # This example focuses on the internal representation of `payload` + # as inferred from supplied types and default values. + # + # We cannot compare `typeof` against `property!` since it excludes `Nil` + # from the possible types. it "makes optional to provide a payload" do - node = Node.new("abc") + node = Node(Int32).new("abc") node.payload?.should be_falsey + typeof(node.@payload).should eq(Int32 | Nil) end end describe "#priority" do it "calculates it based on key size" do - node = Node.new("a") + node = Node(Nil).new("a") node.priority.should eq(1) - node = Node.new("abc") + node = Node(Nil).new("abc") node.priority.should eq(3) end it "returns zero for catch all (globbed) key" do - node = Node.new("*filepath") + node = Node(Nil).new("*filepath") node.priority.should eq(0) - node = Node.new("/src/*filepath") + node = Node(Nil).new("/src/*filepath") node.priority.should eq(0) end it "returns one for keys with named parameters" do - node = Node.new(":query") + node = Node(Nil).new(":query") node.priority.should eq(1) - node = Node.new("/search/:query") + node = Node(Nil).new("/search/:query") node.priority.should eq(1) end it "changes when key changes" do - node = Node.new("a") + node = Node(Nil).new("a") node.priority.should eq(1) node.key = "abc" @@ -71,10 +77,10 @@ module Radix describe "#sort!" do it "orders children by priority" do - root = Node.new("/") - node1 = Node.new("a") - node2 = Node.new("bc") - node3 = Node.new("def") + root = Node(Int32).new("/") + node1 = Node(Int32).new("a", 1) + node2 = Node(Int32).new("bc", 2) + node3 = Node(Int32).new("def", 3) root.children.push(node1, node2, node3) root.sort! @@ -85,10 +91,10 @@ module Radix end it "orders catch all and named parameters lower than others" do - root = Node.new("/") - node1 = Node.new("*filepath") - node2 = Node.new("abc") - node3 = Node.new(":query") + root = Node(Int32).new("/") + node1 = Node(Int32).new("*filepath", 1) + node2 = Node(Int32).new("abc", 2) + node3 = Node(Int32).new(":query", 3) root.children.push(node1, node2, node3) root.sort! diff --git a/spec/radix/result_spec.cr b/spec/radix/result_spec.cr index d45ff87..89ad226 100644 --- a/spec/radix/result_spec.cr +++ b/spec/radix/result_spec.cr @@ -5,15 +5,15 @@ module Radix describe "#found?" do context "a new instance" do it "returns false when no payload is associated" do - result = Result.new + result = Result(Nil).new result.found?.should be_false end end context "with a payload" do it "returns true" do - node = Node.new("/", :root) - result = Result.new + node = Node(Symbol).new("/", :root) + result = Result(Symbol).new result.use node result.found?.should be_true @@ -24,15 +24,15 @@ module Radix describe "#key" do context "a new instance" do it "returns an empty key" do - result = Result.new + result = Result(Nil).new result.key.should eq("") end end context "given one used node" do it "returns the node key" do - node = Node.new("/", :root) - result = Result.new + node = Node(Symbol).new("/", :root) + result = Result(Symbol).new result.use node result.key.should eq("/") @@ -41,9 +41,9 @@ module Radix context "using multiple nodes" do it "combines the node keys" do - node1 = Node.new("/", :root) - node2 = Node.new("about", :about) - result = Result.new + node1 = Node(Symbol).new("/", :root) + node2 = Node(Symbol).new("about", :about) + result = Result(Symbol).new result.use node1 result.use node2 @@ -54,8 +54,8 @@ module Radix describe "#use" do it "uses the node payload" do - node = Node.new("/", :root) - result = Result.new + node = Node(Symbol).new("/", :root) + result = Result(Symbol).new result.payload?.should be_falsey result.use node @@ -64,8 +64,8 @@ module Radix end it "allow not to assign payload" do - node = Node.new("/", :root) - result = Result.new + node = Node(Symbol).new("/", :root) + result = Result(Symbol).new result.payload?.should be_falsey result.use node, payload: false diff --git a/spec/radix/tree_spec.cr b/spec/radix/tree_spec.cr index 60e5f8c..4e5f942 100644 --- a/spec/radix/tree_spec.cr +++ b/spec/radix/tree_spec.cr @@ -3,7 +3,7 @@ require "../spec_helper" # Silence deprecation warnings when running specs and allow # capture them for inspection. module Radix - class Tree + class Tree(T) @show_deprecations = false @stderr : MemoryIO? @@ -20,12 +20,15 @@ module Radix end end +# Simple Payload class +record Payload + module Radix describe Tree do context "a new instance" do it "contains a root placeholder node" do - tree = Tree.new - tree.root.should be_a(Node) + tree = Tree(Symbol).new + tree.root.should be_a(Node(Symbol)) tree.root.payload?.should be_falsey tree.root.placeholder?.should be_true end @@ -34,9 +37,9 @@ module Radix describe "#add" do context "on a new instance" do it "replaces placeholder with new node" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/abc", :abc - tree.root.should be_a(Node) + tree.root.should be_a(Node(Symbol)) tree.root.placeholder?.should be_false tree.root.payload?.should be_truthy tree.root.payload.should eq(:abc) @@ -45,7 +48,7 @@ module Radix context "shared root" do it "inserts properly adjacent nodes" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/a", :a tree.add "/bc", :bc @@ -61,7 +64,7 @@ module Radix end it "inserts nodes with shared parent" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/abc", :abc tree.add "/axyz", :axyz @@ -78,7 +81,7 @@ module Radix end it "inserts multiple parent nodes" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/admin/users", :users tree.add "/admin/products", :products @@ -107,7 +110,7 @@ module Radix end it "inserts multiple nodes with mixed parents" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/authorizations", :authorizations tree.add "/authorizations/:id", :authorization tree.add "/applications", :applications @@ -127,7 +130,7 @@ module Radix end it "supports insertion of mixed routes out of order" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/user/repos", :my_repos tree.add "/users/:user/repos", :user_repos tree.add "/users/:user", :user @@ -148,9 +151,30 @@ module Radix end end + context "mixed payloads" do + it "allows node with different payloads" do + payload1 = Payload.new + payload2 = Payload.new + + tree = Tree(Payload | Symbol).new + tree.add "/", :root + tree.add "/a", payload1 + tree.add "/bc", payload2 + + # / (:root) + # +-bc (payload2) + # \-a (payload1) + tree.root.children.size.should eq(2) + tree.root.children[0].key.should eq("bc") + tree.root.children[0].payload.should eq(payload2) + tree.root.children[1].key.should eq("a") + tree.root.children[1].payload.should eq(payload1) + end + end + context "dealing with duplicates" do it "does not allow same path be defined twice" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/abc", :abc @@ -164,7 +188,7 @@ module Radix context "dealing with catch all and named parameters" do it "prioritizes nodes correctly" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/*filepath", :all tree.add "/products", :products @@ -193,7 +217,7 @@ module Radix end it "does not split named parameters across shared key" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/:category", :category tree.add "/:category/:subcategory", :subcategory @@ -210,7 +234,7 @@ module Radix end it "does not allow different named parameters sharing same level" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/:post", :post @@ -224,7 +248,7 @@ module Radix describe "#find" do context "a single node" do it "does not find when using different path" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/about", :about result = tree.find "/products" @@ -232,7 +256,7 @@ module Radix end it "finds when using matching path" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/about", :about result = tree.find "/about" @@ -243,7 +267,7 @@ module Radix end it "finds when using path with trailing slash" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/about", :about result = tree.find "/about/" @@ -252,7 +276,7 @@ module Radix end it "finds when key has trailing slash" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/about/", :about result = tree.find "/about" @@ -264,7 +288,7 @@ module Radix context "nodes with shared parent" do it "finds matching path" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/abc", :abc tree.add "/axyz", :axyz @@ -276,7 +300,7 @@ module Radix end it "finds matching path across parents" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/admin/users", :users tree.add "/admin/products", :products @@ -292,7 +316,7 @@ module Radix context "dealing with catch all" do it "finds matching path" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/*filepath", :all tree.add "/about", :about @@ -304,7 +328,7 @@ module Radix end it "returns catch all in parameters" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/*filepath", :all tree.add "/about", :about @@ -316,7 +340,7 @@ module Radix end it "returns optional catch all" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/search/*extra", :extra @@ -328,7 +352,7 @@ module Radix end it "does not find when catch all is not full match" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/search/public/*query", :search @@ -339,7 +363,7 @@ module Radix context "dealing with named parameters" do it "finds matching path" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/products", :products tree.add "/products/:id", :product @@ -352,7 +376,7 @@ module Radix end it "does not find partial matching path" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/products", :products tree.add "/products/:id/edit", :edit @@ -362,7 +386,7 @@ module Radix end it "returns named parameters in result" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/products", :products tree.add "/products/:id", :product @@ -375,7 +399,7 @@ module Radix end it "returns unicode values in parameters" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/language/:name", :language tree.add "/language/:name/about", :about @@ -389,7 +413,7 @@ module Radix context "dealing with multiple named parameters" do it "finds matching path" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/:section/:page", :static_page @@ -400,7 +424,7 @@ module Radix end it "returns named parameters in result" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/:section/:page", :static_page @@ -417,7 +441,7 @@ module Radix context "dealing with both catch all and named parameters" do it "finds matching path" do - tree = Tree.new + tree = Tree(Symbol).new tree.add "/", :root tree.add "/*filepath", :all tree.add "/products", :products diff --git a/src/radix/node.cr b/src/radix/node.cr index 28e83cf..21b4e16 100644 --- a/src/radix/node.cr +++ b/src/radix/node.cr @@ -26,11 +26,11 @@ module Radix # node.children.map &.priority # # => [3, 2, 1] # ``` - class Node - getter key : String - getter? placeholder : Bool - property! payload - property children : Array(Node) + class Node(T) + getter key + getter? placeholder + property! payload : T | Nil + property children # Returns the priority of the Node based on it's *key* # @@ -42,16 +42,16 @@ module Radix # * Any other type of key will receive priority based on its size. # # ``` - # Node.new("a").priority + # Node(Nil).new("a").priority # # => 1 # - # Node.new("abc").priority + # Node(Nil).new("abc").priority # # => 3 # - # Node.new("*filepath").priority + # Node(Nil).new("*filepath").priority # # => 0 # - # Node.new(":query").priority + # Node(Nil).new(":query").priority # # => 1 # ``` getter priority : Int32 @@ -59,16 +59,30 @@ module Radix # Instantiate a Node # # - *key* - A `String` that represents this node. - # - *payload* - An Optional payload for this node. - def initialize(@key : String, @payload = nil, @placeholder = false) - @children = [] of Node + # - *payload* - An optional payload for this node. + # + # When *payload* is not supplied, ensure the type of the node is provided + # instead: + # + # ``` + # # Good, node type is inferred from payload (Symbol) + # node = Node.new("/", :root) + # + # # Good, node type is now Int32 but payload is optional + # node = Node(Int32).new("/") + # + # # Error, node type cannot be inferred (compiler error) + # node = Node.new("/") + # ``` + def initialize(@key : String, @payload : T? = nil, @placeholder = false) + @children = [] of Node(T) @priority = compute_priority end # Changes current *key* # # ``` - # node = Node.new("a") + # node = Node(Nil).new("a") # node.key # # => "a" # @@ -80,7 +94,7 @@ module Radix # This will also result in a new priority for the node. # # ``` - # node = Node.new("a") + # node = Node(Nil).new("a") # node.priority # # => 1 # @@ -88,7 +102,7 @@ module Radix # node.priority # # => 6 # ``` - def key=(@key : String) + def key=(@key) @priority = compute_priority end @@ -115,11 +129,11 @@ module Radix # This ensures highest priority nodes are listed before others. # # ``` - # root = Node.new("/") - # root.children << Node.new("*filepath") # node.priority => 0 - # root.children << Node.new(":query") # node.priority => 1 - # root.children << Node.new("a") # node.priority => 1 - # root.children << Node.new("bc") # node.priority => 2 + # root = Node(Nil).new("/") + # root.children << Node(Nil).new("*filepath") # node.priority => 0 + # root.children << Node(Nil).new(":query") # node.priority => 1 + # root.children << Node(Nil).new("a") # node.priority => 1 + # root.children << Node(Nil).new("bc") # node.priority => 2 # root.sort! # # root.children.map &.priority diff --git a/src/radix/result.cr b/src/radix/result.cr index ca93480..b81642d 100644 --- a/src/radix/result.cr +++ b/src/radix/result.cr @@ -12,13 +12,15 @@ module Radix # # A Result is also used recursively by `Tree#find` when collecting extra # information like *params*. - class Result + class Result(T) + @key : String? + getter params - getter! payload + getter! payload : T? # :nodoc: def initialize - @nodes = [] of Node + @nodes = [] of Node(T) @params = {} of String => String end @@ -26,26 +28,26 @@ module Radix # the result. # # ``` - # result = Result.new + # result = Result(Symbol).new # result.found? # # => false # - # root = Node.new("/", :root) + # root = Node(Symbol).new("/", :root) # result.use(root) # result.found? # # => true # ``` - def found? : Bool + def found? payload? ? true : false end # Returns a String built based on the nodes used in the result # # ``` - # node1 = Node.new("/", :root) - # node2 = Node.new("about", :about) + # node1 = Node(Symbol).new("/", :root) + # node2 = Node(Symbol).new("about", :about) # - # result = Result.new + # result = Result(Symbol).new # result.use node1 # result.use node2 # @@ -56,11 +58,11 @@ module Radix # When no node has been used, returns an empty String. # # ``` - # result = Result.new + # result = Result(Nil).new # result.key # # => "" # ``` - def key : String + def key @key ||= begin String.build { |io| @nodes.each do |node| @@ -74,7 +76,7 @@ module Radix # # * Collect `Node` for future references. # * Use *payload* if present. - def use(node : Node, payload = true) + def use(node : Node(T), payload = true) # collect nodes @nodes << node diff --git a/src/radix/tree.cr b/src/radix/tree.cr index fd44954..b43ce29 100644 --- a/src/radix/tree.cr +++ b/src/radix/tree.cr @@ -12,7 +12,7 @@ module Radix # # You can associate a *payload* at insertion which will be return back # at retrieval time. - class Tree + class Tree(T) # :nodoc: class DuplicateError < Exception def initialize(path) @@ -30,10 +30,10 @@ module Radix # Returns the root `Node` element of the Tree. # # On a new tree instance, this will be a placeholder. - getter root : Node + getter root : Node(T) def initialize - @root = Node.new("", placeholder: true) + @root = Node(T).new("", placeholder: true) end # Inserts given *path* into the Tree @@ -98,19 +98,19 @@ module Radix # # \-*filepath (:all) # tree.add "/about", :about # ``` - def add(path : String, payload) + def add(path : String, payload : T) root = @root # replace placeholder with new node if root.placeholder? - @root = Node.new(path, payload) + @root = Node(T).new(path, payload) else add path, payload, root end end # :nodoc: - private def add(path : String, payload, node : Node) + private def add(path : String, payload : T, node : Node(T)) key_reader = Char::Reader.new(node.key) path_reader = Char::Reader.new(path) @@ -151,7 +151,7 @@ module Radix # if no existing child shared part of the key, add a new one unless added - node.children << Node.new(new_key, payload) + node.children << Node(T).new(new_key, payload) end # adjust priorities @@ -174,7 +174,7 @@ module Radix new_key = node.key.byte_slice(path_reader.pos) swap_payload = node.payload? ? node.payload : nil - new_node = Node.new(new_key, swap_payload) + new_node = Node(T).new(new_key, swap_payload) new_node.children.replace(node.children) # clear payload and children (this is no longer and endpoint) @@ -189,7 +189,7 @@ module Radix # determine if path still continues if path_reader.pos < path.size new_key = path.byte_slice(path_reader.pos) - node.children << Node.new(new_key, payload) + node.children << Node(T).new(new_key, payload) node.sort! # clear payload (no endpoint) @@ -223,7 +223,7 @@ module Radix # # => :about # ``` def find(path : String) - result = Result.new + result = Result(T).new root = @root # walk the tree from root (first time)