Merge pull request #28 from luislavena/fix/remove-unused-key

Remove `Radix::Result#key` usage in the codebase

CHANGELOG.md

@@ -6,6 +6,9 @@ so please check *Changed* and *Removed* notes before upgrading.
 
 ## [Unreleased]
 
+### Removed
+- Remove `Radix::Result#key` since exposes internal details about structure (breaking change)
+
 ## [0.3.9] - 2019-01-02
 ### Fixed
 - Correct catch-all issue caused when paths differ [#26](https://github.com/luislavena/radix/pull/26) (@silasb)

spec/radix/result_spec.cr

@@ -21,37 +21,6 @@ module Radix
       end
     end
 
-    describe "#key" do
-      context "a new instance" do
-        it "returns an empty key" do
-          result = Result(Nil).new
-          result.key.should eq("")
-        end
-      end
-
-      context "given one used node" do
-        it "returns the node key" do
-          node = Node(Symbol).new("/", :root)
-          result = Result(Symbol).new
-          result.use node
-
-          result.key.should eq("/")
-        end
-      end
-
-      context "using multiple nodes" do
-        it "combines the node keys" do
-          node1 = Node(Symbol).new("/", :root)
-          node2 = Node(Symbol).new("about", :about)
-          result = Result(Symbol).new
-          result.use node1
-          result.use node2
-
-          result.key.should eq("/about")
-        end
-      end
-    end
-
     describe "#use" do
       it "uses the node payload" do
         node = Node(Symbol).new("/", :root)

spec/radix/tree_spec.cr

@@ -297,7 +297,6 @@ module Radix
 
           result = tree.find "/about"
           result.found?.should be_true
-          result.key.should eq("/about")
           result.payload?.should be_truthy
           result.payload.should eq(:about)
         end
@@ -308,7 +307,6 @@ module Radix
 
           result = tree.find "/about/"
           result.found?.should be_true
-          result.key.should eq("/about")
         end
 
         it "finds when key contains trailing slash" do
@@ -317,7 +315,6 @@ module Radix
 
           result = tree.find "/about"
           result.found?.should be_true
-          result.key.should eq("/about/")
           result.payload.should eq(:about)
         end
       end
@@ -331,7 +328,6 @@ module Radix
 
           result = tree.find("/abc")
           result.found?.should be_true
-          result.key.should eq("/abc")
           result.payload.should eq(:abc)
         end
 
@@ -342,7 +338,6 @@ module Radix
 
           result = tree.find("/products")
           result.found?.should be_true
-          result.key.should eq("/products")
           result.payload.should eq(:products)
         end
 
@@ -356,7 +351,6 @@ module Radix
 
           result = tree.find("/blog/tags/")
           result.found?.should be_true
-          result.key.should eq("/blog/tags")
           result.payload.should eq(:tags)
         end
       end
@@ -370,7 +364,7 @@ module Radix
 
           result = tree.find("/日本日本語は難しい/")
           result.found?.should be_true
-          result.key.should eq("/日本日本語は難しい")
+          result.payload.should eq(:japanese_is_difficult)
         end
       end
 
@@ -383,7 +377,6 @@ module Radix
 
           result = tree.find("/src/file.png")
           result.found?.should be_true
-          result.key.should eq("/*filepath")
           result.payload.should eq(:all)
         end
 
@@ -406,7 +399,6 @@ module Radix
 
           result = tree.find("/search")
           result.found?.should be_true
-          result.key.should eq("/search/*extra")
           result.params.has_key?("extra").should be_true
           result.params["extra"].empty?.should be_true
         end
@@ -417,7 +409,6 @@ module Radix
 
           result = tree.find("/members")
           result.found?.should be_true
-          result.key.should eq("/members*trailing")
           result.params.has_key?("trailing").should be_true
           result.params["trailing"].empty?.should be_true
         end
@@ -446,7 +437,7 @@ module Radix
 
           result = tree.find("/members")
           result.found?.should be_true
-          result.key.should eq("/members")
+          result.payload.should eq(:members)
         end
 
         it "does prefer catch all over specific key with partially shared key" do
@@ -456,7 +447,7 @@ module Radix
 
           result = tree.find("/orders/cancelled")
           result.found?.should be_true
-          result.key.should eq("/orders/*anything")
+          result.payload.should eq(:orders_catch_all)
           result.params.has_key?("anything").should be_true
           result.params["anything"].should eq("cancelled")
         end
@@ -472,7 +463,6 @@ module Radix
 
           result = tree.find("/products/10")
           result.found?.should be_true
-          result.key.should eq("/products/:id")
           result.payload.should eq(:product)
         end
 
@@ -518,7 +508,7 @@ module Radix
 
           result = tree.find("/tag-edit2")
           result.found?.should be_true
-          result.key.should eq("/tag-edit2")
+          result.payload.should eq(:alternate_tag_edit)
         end
 
         it "does prefer named parameter over specific key with partially shared key" do
@@ -528,7 +518,7 @@ module Radix
 
           result = tree.find("/orders/10")
           result.found?.should be_true
-          result.key.should eq("/orders/:id")
+          result.payload.should eq(:specific_order)
           result.params.has_key?("id").should be_true
           result.params["id"].should eq("10")
         end
@@ -542,7 +532,6 @@ module Radix
 
           result = tree.find("/about/shipping")
           result.found?.should be_true
-          result.key.should eq("/:section/:page")
           result.payload.should eq(:static_page)
         end
 
@@ -574,17 +563,16 @@ module Radix
 
           result = tree.find("/products/1000")
           result.found?.should be_true
-          result.key.should eq("/products/:id")
           result.payload.should eq(:product)
 
           result = tree.find("/admin/articles")
           result.found?.should be_true
-          result.key.should eq("/*filepath")
+          result.payload.should eq(:all)
           result.params["filepath"].should eq("admin/articles")
 
           result = tree.find("/products/featured")
           result.found?.should be_true
-          result.key.should eq("/products/featured")
+          result.payload.should eq(:featured)
           result.payload.should eq(:featured)
         end
       end
@@ -597,7 +585,7 @@ module Radix
 
           result = tree.find "/one-longer/10"
           result.found?.should be_true
-          result.key.should eq("/one-longer/:id")
+          result.payload.should eq(:two)
           result.params["id"].should eq("10")
         end
       end

src/radix/result.cr

@@ -1,11 +1,11 @@
 require "./node"
 
 module Radix
-  # A Result is the comulative output of walking our [Radix tree](https://en.wikipedia.org/wiki/Radix_tree)
+  # Result present the output of walking our [Radix tree](https://en.wikipedia.org/wiki/Radix_tree)
   # `Radix::Tree` implementation.
   #
-  # It provides helpers to retrieve the information obtained from walking
+  # It provides helpers to retrieve the success (or failure) and the payload
-  # our tree using `Radix::Tree#find`
+  # obtained from walkin our tree using `Radix::Tree#find`
   #
   # This information can be used to perform actions in case of the *path*
   # that was looked on the Tree was found.
@@ -20,7 +20,6 @@ module Radix
 
     # :nodoc:
     def initialize
-      @nodes = [] of Node(T)
       @params = {} of String => String
     end
 
@@ -41,45 +40,11 @@ module Radix
       payload? ? true : false
     end
 
-    # Returns a String built based on the nodes used in the result
-    #
-    # ```
-    # node1 = Radix::Node(Symbol).new("/", :root)
-    # node2 = Radix::Node(Symbol).new("about", :about)
-    #
-    # result = Radix::Result(Symbol).new
-    # result.use node1
-    # result.use node2
-    #
-    # result.key
-    # # => "/about"
-    # ```
-    #
-    # When no node has been used, returns an empty String.
-    #
-    # ```
-    # result = Radix::Result(Nil).new
-    # result.key
-    # # => ""
-    # ```
-    def key
-      @key ||= begin
-        String.build { |io|
-          @nodes.each do |node|
-            io << node.key
-          end
-        }
-      end
-    end
-
     # Adjust result information by using the details of the given `Node`.
     #
     # * Collect `Node` for future references.
     # * Use *payload* if present.
     def use(node : Node(T), payload = true)
-      # collect nodes
-      @nodes << node
-
       if payload && node.payload?
         @payload = node.payload
       end