From e6af09c8d9a01d6fca1b61ba6feb4a0f8a12be7e Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Fri, 14 Jun 2019 14:50:24 -0500
Subject: [PATCH 1/8] Add CleverDicts submodule

---
 Project.toml                  |   3 +-
 src/Utilities/CleverDicts.jl  | 193 ++++++++++++++++++++++++++++++++++
 src/Utilities/Utilities.jl    |   2 +
 test/Utilities/CleverDicts.jl | 142 +++++++++++++++++++++++++
 test/Utilities/Utilities.jl   |   4 +
 5 files changed, 343 insertions(+), 1 deletion(-)
 create mode 100644 src/Utilities/CleverDicts.jl
 create mode 100644 test/Utilities/CleverDicts.jl

diff --git a/Project.toml b/Project.toml
index a48c7afbe3..167c9a3a5c 100644
--- a/Project.toml
+++ b/Project.toml
@@ -5,9 +5,10 @@ version = "0.9.0"
 [deps]
 BenchmarkTools = "6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf"
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
+OrderedCollections = "bac558e1-5e72-5ebc-8fee-abe8a469f55d"
 SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
-Unicode = "4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+Unicode = "4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5"
 
 [compat]
 julia = "1"
diff --git a/src/Utilities/CleverDicts.jl b/src/Utilities/CleverDicts.jl
new file mode 100644
index 0000000000..5c2808da2a
--- /dev/null
+++ b/src/Utilities/CleverDicts.jl
@@ -0,0 +1,193 @@
+module CleverDicts
+
+import OrderedCollections
+
+"""
+    CleverDict{K, V}
+
+A smart storage type for managing sequential objects with non-decreasing integer
+indices.
+
+Provided no keys are deleted, the backing storage is a `Vector{V}`. Once a key
+has been deleted, the backing storage switches to an `OrderedDict{K, V}`.
+
+The i'th ordered element can be obtained with `c[LinearIndex(i)]`.
+
+Note that querying a `LinearIndex` immediately after deleting a key via
+`delete!` is very slow. (It requires a rebuild of an ordered list of variables.)
+
+Use `new_key(c::CleverDict)` to obtain the next key in the sequential order.
+
+Overload the functions `index_to_key` and `key_to_index` to enable mappings
+between the integer index of the vector and the dictionary key.
+"""
+mutable struct CleverDict{K, V}
+    last_index::Int
+    vector::Union{Nothing, Vector{V}}
+    dict::Union{Nothing, OrderedCollections.OrderedDict{K, V}}
+    CleverDict{K, V}() where {K, V} = new{K, V}(0, V[], nothing)
+end
+
+"""
+    index_to_key(::Type{K}, index::Int)
+
+Create a new key associated with the integer value `index`.
+"""
+function index_to_key(::Type{K}, index::Int) where {K}
+    error("You need to define `index_to_key`.")
+end
+
+"""
+    key_to_index(key::K)
+
+Map `key` to an integer valued index, assuming that there have been no
+deletions.
+"""
+function key_to_index(key::K) where {K}
+    error("You need to define `key_to_index`.")
+end
+
+"""
+    new_item(c::CleverDict{K, V}, val::Val)::K where {K, V}
+
+Set `val` in the next available key, and return that key.
+"""
+function new_item(c::CleverDict{K, V}, val::V)::K where {K, V}
+    c.last_index += 1
+    key = index_to_key(K, c.last_index)
+    if c.dict === nothing
+        push!(c.vector, val)
+    else
+        c.dict[key] = val
+        # If there is a vector (e.g., because it has been rebuild for
+        # `LinearIndex`), clear it.
+        c.vector = nothing
+    end
+    return key
+end
+
+function Base.empty!(c::CleverDict{K, V})::Nothing where {K, V}
+    c.vector = V[]
+    c.last_index = 0
+    c.dict = nothing
+    return
+end
+
+function Base.getindex(c::CleverDict{K, V}, key::K)::V where {K, V}
+    # Perform this `haskey` check up front to detect getting with keys that are
+    # invalid (i.e., have previously been deleted).
+    if !haskey(c, key)
+        throw(KeyError(key))
+    end
+    # Case  I) no call to `Base.delete!`, so return the element:
+    # Case II) `Base.delete!` must have been called, so return the element
+    #          from the dictionary.
+    return c.dict === nothing ? c.vector[key_to_index(key)] : c.dict[key]
+end
+
+function Base.setindex!(c::CleverDict{K, V}, val::V, key::K)::V where {K, V}
+    # Perform this `haskey` check up front to detect setting with keys that are
+    # invalid (i.e., have already been deleted). You can only call setindex!
+    # with a key obtained from `new_key` that hasn't been deleted.
+    if !haskey(c, key)
+        throw(KeyError(key))
+    elseif c.dict === nothing
+        @assert c.vector !== nothing
+        c.vector[key_to_index(key)] = val
+    else
+        c.dict[key] = val
+    end
+    return val
+end
+
+struct LinearIndex
+    i::Int
+end
+
+function Base.getindex(c::CleverDict{K, V}, index::LinearIndex)::V where {K, V}
+    if !(1 <= index.i <= length(c))
+        throw(KeyError(index))
+    end
+    # Get the `index` linear element. If `c.vector` is currently `nothing`
+    # (i.e., there has been a deletion), rebuild `c.vector`. This is a
+    # trade-off: We could ensure `c.vector` is always updated, but this requires
+    # a `splice!` in `delete!`, making deletions costly. However, it makes this
+    # `getindex` operation trival because we would never have to rebuild the
+    # vector.
+    # The current implemented approach offers quick deletions, but an expensive
+    # rebuild the first time you query a `LinearIndex` after a deletion or a new
+    # key is added. Once the rebuild is done, there are quick queries until the
+    # next deletion or addition. Thus, the worst-case is a user repeatedly
+    # deleting a key and then querying a LinearIndex (e.g., getting the MOI
+    # objective function).
+    if c.vector === nothing
+        c.vector = Vector{V}(undef, length(c))
+        for (i, val) in enumerate(values(c.dict))
+            c.vector[i] = val
+        end
+    end
+    return c.vector[index.i]::V
+end
+
+function Base.delete!(c::CleverDict{K, V}, key::K)::Nothing where {K, V}
+    if c.dict === nothing
+        c.dict = OrderedCollections.OrderedDict{K, Union{Nothing, V}}()
+        for (i, info) in enumerate(c.vector)
+            c.dict[index_to_key(K, i)] = info
+        end
+    end
+    delete!(c.dict, key)
+    c.vector = nothing
+    return
+end
+
+function Base.length(c::CleverDict)::Int
+    return c.dict == nothing ? length(c.vector) : length(c.dict)
+end
+
+Base.haskey(::CleverDict, key) = false
+function Base.haskey(c::CleverDict{K, V}, key::K)::Bool where {K, V}
+    if c.dict === nothing
+        return 1 <= key_to_index(key) <= length(c.vector)
+    else
+        return haskey(c.dict, key)
+    end
+end
+
+function Base.iterate(
+    c::CleverDict{K, V}
+)::Union{Nothing, Tuple{Pair{K, V}, Int}} where {K, V}
+    if length(c) == 0
+        return nothing
+    elseif c.dict !== nothing
+        return iterate(c.dict)
+    else
+        @assert c.vector !== nothing
+        key = index_to_key(K, 1)
+        return key => c.vector[1], 2
+    end
+end
+
+function Base.iterate(
+    c::CleverDict{K, V}, s::Int
+)::Union{Nothing, Tuple{Pair{K, V}, Int}} where {K, V}
+    if s > length(c)
+        return nothing
+    elseif c.dict !== nothing
+        return iterate(c.dict, s)
+    else
+        @assert c.vector !== nothing
+        key = index_to_key(K, s)
+        return key => c.vector[s], s + 1
+    end
+end
+
+function Base.values(c::CleverDict{K, V}) where {K, V}
+    return c.dict !== nothing ? values(c.dict) : c.vector
+end
+
+function Base.keys(c::CleverDict{K, V}) where {K, V}
+    return c.dict !== nothing ? keys(c.dict) : index_to_key.(K, 1:length(c))
+end
+
+end
diff --git a/src/Utilities/Utilities.jl b/src/Utilities/Utilities.jl
index b7de6876f9..6ab5ba9c8a 100644
--- a/src/Utilities/Utilities.jl
+++ b/src/Utilities/Utilities.jl
@@ -29,4 +29,6 @@ include("mockoptimizer.jl")
 include("cachingoptimizer.jl")
 include("universalfallback.jl")
 
+include("CleverDicts.jl")
+
 end # module
diff --git a/test/Utilities/CleverDicts.jl b/test/Utilities/CleverDicts.jl
new file mode 100644
index 0000000000..690d9a5569
--- /dev/null
+++ b/test/Utilities/CleverDicts.jl
@@ -0,0 +1,142 @@
+using MathOptInterface, Test
+
+const CleverDicts = MathOptInterface.Utilities.CleverDicts
+
+struct MyKey
+    x::Int
+end
+
+CleverDicts.key_to_index(key::MyKey) = key.x
+CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
+
+@testset "CleverDict" begin
+    @testset "get/set" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        key = CleverDicts.new_item(d, "first")
+        @test key == MyKey(1)
+        @test d[key] == "first"
+        @test haskey(d, key) == true
+        @test_throws KeyError d[MyKey(2)]
+        delete!(d, key)
+        @test_throws KeyError d[key]
+        @test_throws KeyError d[key] = "key"
+        @test haskey(d, key) == false
+        key2 = CleverDicts.new_item(d, "second")
+        @test key2 == MyKey(2)
+        @test d[key2] == "second"
+        @test d.vector === nothing
+        @test d.dict !== nothing
+        d[key2] = "third"
+        @test d[key2] == "third"
+
+        empty!(d)
+
+        key = CleverDicts.new_item(d, "first")
+        @test key == MyKey(1)
+        @test d[key] == "first"
+        d[key] = "zeroth"
+        @test d[key] == "zeroth"
+        @test haskey(d, key) == true
+        @test_throws KeyError d[MyKey(2)]
+        delete!(d, key)
+        @test_throws KeyError d[key]
+        @test_throws KeyError d[key] = "key"
+        @test haskey(d, key) == false
+        key2 = CleverDicts.new_item(d, "second")
+        @test key2 == MyKey(2)
+        @test d[key2] == "second"
+        @test d.vector === nothing
+        @test d.dict !== nothing
+    end
+
+    @testset "LinearIndex" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        key = CleverDicts.new_item(d, "first")
+        @test d[CleverDicts.LinearIndex(1)] == "first"
+        key2 = CleverDicts.new_item(d, "second")
+        @test d[CleverDicts.LinearIndex(2)] == "second"
+        @test length(d) == 2
+        delete!(d, key)
+        @test d.vector === nothing
+        @test d[CleverDicts.LinearIndex(1)] == "second"
+        @test_throws KeyError d[CleverDicts.LinearIndex(2)]
+        @test length(d) == 1
+        @test d.vector !== nothing
+    end
+
+    @testset "keys/values" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        key = CleverDicts.new_item(d, "first")
+        key2 = CleverDicts.new_item(d, "second")
+        @test collect(keys(d)) == [MyKey(1), MyKey(2)]
+        @test collect(values(d)) == ["first", "second"]
+        delete!(d, key)
+        key3 = CleverDicts.new_item(d, "third")
+        @test collect(keys(d)) == [MyKey(2), MyKey(3)]
+        @test collect(values(d)) == ["second", "third"]
+    end
+
+    @testset "iterate" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        key = CleverDicts.new_item(d, "first")
+        key2 = CleverDicts.new_item(d, "second")
+        my_keys = MyKey[]
+        my_values = String[]
+        for (k, v) in d
+           push!(my_keys, k)
+           push!(my_values, v)
+        end
+        @test my_keys == [MyKey(1), MyKey(2)]
+        @test my_values == ["first", "second"]
+        delete!(d, key)
+        key3 = CleverDicts.new_item(d, "third")
+        my_keys = MyKey[]
+        my_values = String[]
+        for (k, v) in d
+           push!(my_keys, k)
+           push!(my_values, v)
+        end
+        @test my_keys == [MyKey(2), MyKey(3)]
+        @test my_values == ["second", "third"]
+    end
+
+    @testset "iterate ii" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        key = CleverDicts.new_item(d, "first")
+        key2 = CleverDicts.new_item(d, "second")
+        my_keys = MyKey[]
+        my_values = String[]
+        for (k, v) in d
+            push!(my_keys, k)
+            push!(my_values, v)
+        end
+        @test my_keys == [MyKey(1), MyKey(2)]
+        @test my_values == ["first", "second"]
+        delete!(d, key)
+        @test d[CleverDicts.LinearIndex(1)] == "second"
+        key3 = CleverDicts.new_item(d, "third")
+        my_keys = MyKey[]
+        my_values = String[]
+        for (k, v) in d
+            push!(my_keys, k)
+            push!(my_values, v)
+        end
+        @test my_keys == [MyKey(2), MyKey(3)]
+        @test my_values == ["second", "third"]
+    end
+
+    @testset "delete!" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        @test length(d) == 0
+        @test delete!(d, MyKey(0)) == nothing
+        k1 = CleverDicts.new_item(d, "a")
+        k2 = CleverDicts.new_item(d, "b")
+        d[CleverDicts.LinearIndex(2)] == "b"
+        delete!(d, k1)
+        d[CleverDicts.LinearIndex(1)] == "b"
+        k3 = CleverDicts.new_item(d, "c")
+        @test d[k3] == "c"
+        @test d[CleverDicts.LinearIndex(1)] == "b"
+        @test d[CleverDicts.LinearIndex(2)] == "c"
+    end
+end
diff --git a/test/Utilities/Utilities.jl b/test/Utilities/Utilities.jl
index 9e8a905f92..df160637c0 100644
--- a/test/Utilities/Utilities.jl
+++ b/test/Utilities/Utilities.jl
@@ -27,3 +27,7 @@ end
 @testset "Copy" begin
     include("copy.jl")
 end
+
+@testset "CleverDicts" begin
+    include("CleverDicts.jl")
+end

From 6b37e3e664b62d043d9f9d3ec0287b2c4f52cd2d Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Fri, 14 Jun 2019 14:54:14 -0500
Subject: [PATCH 2/8] Update docs

---
 src/Utilities/CleverDicts.jl | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/Utilities/CleverDicts.jl b/src/Utilities/CleverDicts.jl
index 5c2808da2a..d3450fdecf 100644
--- a/src/Utilities/CleverDicts.jl
+++ b/src/Utilities/CleverDicts.jl
@@ -16,10 +16,19 @@ The i'th ordered element can be obtained with `c[LinearIndex(i)]`.
 Note that querying a `LinearIndex` immediately after deleting a key via
 `delete!` is very slow. (It requires a rebuild of an ordered list of variables.)
 
-Use `new_key(c::CleverDict)` to obtain the next key in the sequential order.
+Store an item `val` using `new_item(c::CleverDict, val)`. `new_item` returns a
+key corresponding to the stored item.
 
 Overload the functions `index_to_key` and `key_to_index` to enable mappings
 between the integer index of the vector and the dictionary key.
+
+## Example
+
+    struct MyKey
+        x::Int
+    end
+    index_to_key(::Type{MyKey}, i::Int) = MyKey(i)
+    key_to_index(key::MyKey) = key.x
 """
 mutable struct CleverDict{K, V}
     last_index::Int

From 3de72342b8ca329b46c00f0d7b90314306846502 Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Fri, 14 Jun 2019 18:13:26 -0500
Subject: [PATCH 3/8] Improve test coverage

---
 src/Utilities/CleverDicts.jl  |  8 ++------
 test/Utilities/CleverDicts.jl | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/Utilities/CleverDicts.jl b/src/Utilities/CleverDicts.jl
index d3450fdecf..46712759e4 100644
--- a/src/Utilities/CleverDicts.jl
+++ b/src/Utilities/CleverDicts.jl
@@ -42,9 +42,7 @@ end
 
 Create a new key associated with the integer value `index`.
 """
-function index_to_key(::Type{K}, index::Int) where {K}
-    error("You need to define `index_to_key`.")
-end
+function index_to_key end
 
 """
     key_to_index(key::K)
@@ -52,9 +50,7 @@ end
 Map `key` to an integer valued index, assuming that there have been no
 deletions.
 """
-function key_to_index(key::K) where {K}
-    error("You need to define `key_to_index`.")
-end
+function key_to_index end
 
 """
     new_item(c::CleverDict{K, V}, val::Val)::K where {K, V}
diff --git a/test/Utilities/CleverDicts.jl b/test/Utilities/CleverDicts.jl
index 690d9a5569..b5e3b83aea 100644
--- a/test/Utilities/CleverDicts.jl
+++ b/test/Utilities/CleverDicts.jl
@@ -125,6 +125,27 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
         @test my_values == ["second", "third"]
     end
 
+    @testset "iterate iii" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        y = 0
+        for (k, v) in d
+            y += 1
+        end
+        @test y == 0
+    end
+
+    @testset "haskey" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        @test !haskey(d, 1)
+        k = CleverDicts.new_item(d, "a")
+        @test haskey(d, k)
+        j = CleverDicts.new_item(d, "b")
+        @test haskey(d, j)
+        delete!(d, k)
+        @test !haskey(d, k)
+        @test haskey(d, j)
+    end
+
     @testset "delete!" begin
         d = CleverDicts.CleverDict{MyKey, String}()
         @test length(d) == 0

From f41b537642880abc15d175d0d0770e3a0eb9c1e7 Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Sat, 15 Jun 2019 10:55:21 -0500
Subject: [PATCH 4/8] Add comments justifying iteration decisions

---
 src/Utilities/CleverDicts.jl | 37 +++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/Utilities/CleverDicts.jl b/src/Utilities/CleverDicts.jl
index 46712759e4..0f8e52dcdc 100644
--- a/src/Utilities/CleverDicts.jl
+++ b/src/Utilities/CleverDicts.jl
@@ -24,11 +24,13 @@ between the integer index of the vector and the dictionary key.
 
 ## Example
 
-    struct MyKey
-        x::Int
-    end
-    index_to_key(::Type{MyKey}, i::Int) = MyKey(i)
-    key_to_index(key::MyKey) = key.x
+```julia
+struct MyKey
+    x::Int
+end
+index_to_key(::Type{MyKey}, i::Int) = MyKey(i)
+key_to_index(key::MyKey) = key.x
+```
 """
 mutable struct CleverDict{K, V}
     last_index::Int
@@ -159,15 +161,27 @@ function Base.haskey(c::CleverDict{K, V}, key::K)::Bool where {K, V}
     end
 end
 
+# Here, we implement the iterate functions for our `CleverDict`. If the backing
+# datastructure is an `OrderedDict`, we just forward `iterate` to the dict. If
+# it's the vector, we create a key-value pair so that `iterate` returns the same
+# type regardless of the backing datastructure. To help inference, we annotate
+# the return type.
+#
+# Also note that iterating an `OrderedDict` returns an `Int` state variable.
+# This is identical to the type of the state variable that we return when
+# iterating the vector, so we can add a type restriction on
+# `iterate(c, s::Int)`.
+
 function Base.iterate(
     c::CleverDict{K, V}
 )::Union{Nothing, Tuple{Pair{K, V}, Int}} where {K, V}
-    if length(c) == 0
-        return nothing
-    elseif c.dict !== nothing
+    if c.dict !== nothing
         return iterate(c.dict)
     else
         @assert c.vector !== nothing
+        if length(c.vector) == 0
+            return nothing
+        end
         key = index_to_key(K, 1)
         return key => c.vector[1], 2
     end
@@ -176,12 +190,13 @@ end
 function Base.iterate(
     c::CleverDict{K, V}, s::Int
 )::Union{Nothing, Tuple{Pair{K, V}, Int}} where {K, V}
-    if s > length(c)
-        return nothing
-    elseif c.dict !== nothing
+    if c.dict !== nothing
         return iterate(c.dict, s)
     else
         @assert c.vector !== nothing
+        if s > length(c.vector)
+            return nothing
+        end
         key = index_to_key(K, s)
         return key => c.vector[s], s + 1
     end

From 6d133083c23bd7d648fc93feb4c7128b6997805a Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Sat, 15 Jun 2019 11:14:28 -0500
Subject: [PATCH 5/8] Rename new_item -> add_item

---
 src/Utilities/CleverDicts.jl  |  6 +++---
 test/Utilities/CleverDicts.jl | 40 +++++++++++++++++------------------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/Utilities/CleverDicts.jl b/src/Utilities/CleverDicts.jl
index 0f8e52dcdc..c9be94a232 100644
--- a/src/Utilities/CleverDicts.jl
+++ b/src/Utilities/CleverDicts.jl
@@ -16,7 +16,7 @@ The i'th ordered element can be obtained with `c[LinearIndex(i)]`.
 Note that querying a `LinearIndex` immediately after deleting a key via
 `delete!` is very slow. (It requires a rebuild of an ordered list of variables.)
 
-Store an item `val` using `new_item(c::CleverDict, val)`. `new_item` returns a
+Store an item `val` using `add_item(c::CleverDict, val)`. `add_item` returns a
 key corresponding to the stored item.
 
 Overload the functions `index_to_key` and `key_to_index` to enable mappings
@@ -55,11 +55,11 @@ deletions.
 function key_to_index end
 
 """
-    new_item(c::CleverDict{K, V}, val::Val)::K where {K, V}
+    add_item(c::CleverDict{K, V}, val::Val)::K where {K, V}
 
 Set `val` in the next available key, and return that key.
 """
-function new_item(c::CleverDict{K, V}, val::V)::K where {K, V}
+function add_item(c::CleverDict{K, V}, val::V)::K where {K, V}
     c.last_index += 1
     key = index_to_key(K, c.last_index)
     if c.dict === nothing
diff --git a/test/Utilities/CleverDicts.jl b/test/Utilities/CleverDicts.jl
index b5e3b83aea..819653e1c5 100644
--- a/test/Utilities/CleverDicts.jl
+++ b/test/Utilities/CleverDicts.jl
@@ -12,7 +12,7 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
 @testset "CleverDict" begin
     @testset "get/set" begin
         d = CleverDicts.CleverDict{MyKey, String}()
-        key = CleverDicts.new_item(d, "first")
+        key = CleverDicts.add_item(d, "first")
         @test key == MyKey(1)
         @test d[key] == "first"
         @test haskey(d, key) == true
@@ -21,7 +21,7 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
         @test_throws KeyError d[key]
         @test_throws KeyError d[key] = "key"
         @test haskey(d, key) == false
-        key2 = CleverDicts.new_item(d, "second")
+        key2 = CleverDicts.add_item(d, "second")
         @test key2 == MyKey(2)
         @test d[key2] == "second"
         @test d.vector === nothing
@@ -31,7 +31,7 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
 
         empty!(d)
 
-        key = CleverDicts.new_item(d, "first")
+        key = CleverDicts.add_item(d, "first")
         @test key == MyKey(1)
         @test d[key] == "first"
         d[key] = "zeroth"
@@ -42,7 +42,7 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
         @test_throws KeyError d[key]
         @test_throws KeyError d[key] = "key"
         @test haskey(d, key) == false
-        key2 = CleverDicts.new_item(d, "second")
+        key2 = CleverDicts.add_item(d, "second")
         @test key2 == MyKey(2)
         @test d[key2] == "second"
         @test d.vector === nothing
@@ -51,9 +51,9 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
 
     @testset "LinearIndex" begin
         d = CleverDicts.CleverDict{MyKey, String}()
-        key = CleverDicts.new_item(d, "first")
+        key = CleverDicts.add_item(d, "first")
         @test d[CleverDicts.LinearIndex(1)] == "first"
-        key2 = CleverDicts.new_item(d, "second")
+        key2 = CleverDicts.add_item(d, "second")
         @test d[CleverDicts.LinearIndex(2)] == "second"
         @test length(d) == 2
         delete!(d, key)
@@ -66,20 +66,20 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
 
     @testset "keys/values" begin
         d = CleverDicts.CleverDict{MyKey, String}()
-        key = CleverDicts.new_item(d, "first")
-        key2 = CleverDicts.new_item(d, "second")
+        key = CleverDicts.add_item(d, "first")
+        key2 = CleverDicts.add_item(d, "second")
         @test collect(keys(d)) == [MyKey(1), MyKey(2)]
         @test collect(values(d)) == ["first", "second"]
         delete!(d, key)
-        key3 = CleverDicts.new_item(d, "third")
+        key3 = CleverDicts.add_item(d, "third")
         @test collect(keys(d)) == [MyKey(2), MyKey(3)]
         @test collect(values(d)) == ["second", "third"]
     end
 
     @testset "iterate" begin
         d = CleverDicts.CleverDict{MyKey, String}()
-        key = CleverDicts.new_item(d, "first")
-        key2 = CleverDicts.new_item(d, "second")
+        key = CleverDicts.add_item(d, "first")
+        key2 = CleverDicts.add_item(d, "second")
         my_keys = MyKey[]
         my_values = String[]
         for (k, v) in d
@@ -89,7 +89,7 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
         @test my_keys == [MyKey(1), MyKey(2)]
         @test my_values == ["first", "second"]
         delete!(d, key)
-        key3 = CleverDicts.new_item(d, "third")
+        key3 = CleverDicts.add_item(d, "third")
         my_keys = MyKey[]
         my_values = String[]
         for (k, v) in d
@@ -102,8 +102,8 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
 
     @testset "iterate ii" begin
         d = CleverDicts.CleverDict{MyKey, String}()
-        key = CleverDicts.new_item(d, "first")
-        key2 = CleverDicts.new_item(d, "second")
+        key = CleverDicts.add_item(d, "first")
+        key2 = CleverDicts.add_item(d, "second")
         my_keys = MyKey[]
         my_values = String[]
         for (k, v) in d
@@ -114,7 +114,7 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
         @test my_values == ["first", "second"]
         delete!(d, key)
         @test d[CleverDicts.LinearIndex(1)] == "second"
-        key3 = CleverDicts.new_item(d, "third")
+        key3 = CleverDicts.add_item(d, "third")
         my_keys = MyKey[]
         my_values = String[]
         for (k, v) in d
@@ -137,9 +137,9 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
     @testset "haskey" begin
         d = CleverDicts.CleverDict{MyKey, String}()
         @test !haskey(d, 1)
-        k = CleverDicts.new_item(d, "a")
+        k = CleverDicts.add_item(d, "a")
         @test haskey(d, k)
-        j = CleverDicts.new_item(d, "b")
+        j = CleverDicts.add_item(d, "b")
         @test haskey(d, j)
         delete!(d, k)
         @test !haskey(d, k)
@@ -150,12 +150,12 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
         d = CleverDicts.CleverDict{MyKey, String}()
         @test length(d) == 0
         @test delete!(d, MyKey(0)) == nothing
-        k1 = CleverDicts.new_item(d, "a")
-        k2 = CleverDicts.new_item(d, "b")
+        k1 = CleverDicts.add_item(d, "a")
+        k2 = CleverDicts.add_item(d, "b")
         d[CleverDicts.LinearIndex(2)] == "b"
         delete!(d, k1)
         d[CleverDicts.LinearIndex(1)] == "b"
-        k3 = CleverDicts.new_item(d, "c")
+        k3 = CleverDicts.add_item(d, "c")
         @test d[k3] == "c"
         @test d[CleverDicts.LinearIndex(1)] == "b"
         @test d[CleverDicts.LinearIndex(2)] == "c"

From 3a5b0dd7c81d4584293ac4fff68578a288550046 Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Sat, 15 Jun 2019 11:23:32 -0500
Subject: [PATCH 6/8] Add isempty

---
 src/Utilities/CleverDicts.jl  |  4 ++++
 test/Utilities/CleverDicts.jl | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/src/Utilities/CleverDicts.jl b/src/Utilities/CleverDicts.jl
index c9be94a232..919799aa8a 100644
--- a/src/Utilities/CleverDicts.jl
+++ b/src/Utilities/CleverDicts.jl
@@ -152,6 +152,10 @@ function Base.length(c::CleverDict)::Int
     return c.dict == nothing ? length(c.vector) : length(c.dict)
 end
 
+function Base.isempty(c::CleverDict)
+    return c.dict !== nothing ? isempty(c.dict) : length(c.vector) == 0
+end
+
 Base.haskey(::CleverDict, key) = false
 function Base.haskey(c::CleverDict{K, V}, key::K)::Bool where {K, V}
     if c.dict === nothing
diff --git a/test/Utilities/CleverDicts.jl b/test/Utilities/CleverDicts.jl
index 819653e1c5..899bd6d055 100644
--- a/test/Utilities/CleverDicts.jl
+++ b/test/Utilities/CleverDicts.jl
@@ -146,6 +146,17 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
         @test haskey(d, j)
     end
 
+    @testset "haskey" begin
+        d = CleverDicts.CleverDict{MyKey, String}()
+        @test isempty(d) == true
+        k = CleverDicts.add_item(d, "a")
+        @test isempty(d) == false
+        delete!(d, k)
+        @test isempty(d) == true
+        j = CleverDicts.add_item(d, "b")
+        @test isempty(d) == false
+    end
+
     @testset "delete!" begin
         d = CleverDicts.CleverDict{MyKey, String}()
         @test length(d) == 0

From bb536a8c437827a43ee45276ba972612de079ada Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Sat, 15 Jun 2019 16:23:48 -0500
Subject: [PATCH 7/8] Use isempty(c.vector) and standardize on c.dict ===
 nothing

---
 src/Utilities/CleverDicts.jl | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/Utilities/CleverDicts.jl b/src/Utilities/CleverDicts.jl
index 919799aa8a..60a3703e57 100644
--- a/src/Utilities/CleverDicts.jl
+++ b/src/Utilities/CleverDicts.jl
@@ -149,11 +149,11 @@ function Base.delete!(c::CleverDict{K, V}, key::K)::Nothing where {K, V}
 end
 
 function Base.length(c::CleverDict)::Int
-    return c.dict == nothing ? length(c.vector) : length(c.dict)
+    return c.dict === nothing ? length(c.vector) : length(c.dict)
 end
 
 function Base.isempty(c::CleverDict)
-    return c.dict !== nothing ? isempty(c.dict) : length(c.vector) == 0
+    return c.dict === nothing ? isempty(c.vector) : isempty(c.dict)
 end
 
 Base.haskey(::CleverDict, key) = false
@@ -179,39 +179,39 @@ end
 function Base.iterate(
     c::CleverDict{K, V}
 )::Union{Nothing, Tuple{Pair{K, V}, Int}} where {K, V}
-    if c.dict !== nothing
-        return iterate(c.dict)
-    else
+    if c.dict === nothing
         @assert c.vector !== nothing
-        if length(c.vector) == 0
+        if isempty(c.vector)
             return nothing
         end
         key = index_to_key(K, 1)
         return key => c.vector[1], 2
+    else
+        return iterate(c.dict)
     end
 end
 
 function Base.iterate(
     c::CleverDict{K, V}, s::Int
 )::Union{Nothing, Tuple{Pair{K, V}, Int}} where {K, V}
-    if c.dict !== nothing
-        return iterate(c.dict, s)
-    else
+    if c.dict === nothing
         @assert c.vector !== nothing
         if s > length(c.vector)
             return nothing
         end
         key = index_to_key(K, s)
         return key => c.vector[s], s + 1
+    else
+        return iterate(c.dict, s)
     end
 end
 
 function Base.values(c::CleverDict{K, V}) where {K, V}
-    return c.dict !== nothing ? values(c.dict) : c.vector
+    return c.dict === nothing ? c.vector : values(c.dict)
 end
 
 function Base.keys(c::CleverDict{K, V}) where {K, V}
-    return c.dict !== nothing ? keys(c.dict) : index_to_key.(K, 1:length(c))
+    return c.dict === nothing ? index_to_key.(K, 1:length(c)) : keys(c.dict)
 end
 
 end

From f5285e0cdab95bda84d5c4ac6efd24a49dcce4f5 Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Sat, 15 Jun 2019 19:46:04 -0500
Subject: [PATCH 8/8] Overload MOI.VariableIndex as key for CleverDict

---
 src/Utilities/CleverDicts.jl  | 14 +++++++
 test/Utilities/CleverDicts.jl | 70 +++++++++++++++++++----------------
 2 files changed, 52 insertions(+), 32 deletions(-)

diff --git a/src/Utilities/CleverDicts.jl b/src/Utilities/CleverDicts.jl
index 60a3703e57..ce6b21adb1 100644
--- a/src/Utilities/CleverDicts.jl
+++ b/src/Utilities/CleverDicts.jl
@@ -1,5 +1,19 @@
 module CleverDicts
 
+# The following two functions are overloaded for `MOI.VariableIndex` here
+# it is the original use-case for `CleverDict`, and it would be type-piracy for
+# solvers using `CleverDicts` to implement it themselves.
+
+import MathOptInterface
+
+function index_to_key(::Type{MathOptInterface.VariableIndex}, index::Int)
+    return MathOptInterface.VariableIndex(index)
+end
+
+key_to_index(key::MathOptInterface.VariableIndex) = key.value
+
+# Now, on with `CleverDicts`.
+
 import OrderedCollections
 
 """
diff --git a/test/Utilities/CleverDicts.jl b/test/Utilities/CleverDicts.jl
index 899bd6d055..d68b333b90 100644
--- a/test/Utilities/CleverDicts.jl
+++ b/test/Utilities/CleverDicts.jl
@@ -2,27 +2,33 @@ using MathOptInterface, Test
 
 const CleverDicts = MathOptInterface.Utilities.CleverDicts
 
-struct MyKey
-    x::Int
-end
-
-CleverDicts.key_to_index(key::MyKey) = key.x
-CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
+# Note: `MyKey` is just for testing. You wouldn't want to use it in practice
+# because the key type of the dictionary isn't a concrete type.
+struct MyKey{X} end
+CleverDicts.key_to_index(::MyKey{X}) where {X} = X
+CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey{index}()
 
 @testset "CleverDict" begin
-    @testset "get/set" begin
+    @testset "MyKey type" begin
         d = CleverDicts.CleverDict{MyKey, String}()
         key = CleverDicts.add_item(d, "first")
-        @test key == MyKey(1)
+        @test key == MyKey{1}()
+        @test d[MyKey{1}()] == "first"
+    end
+
+    @testset "get/set" begin
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
+        key = CleverDicts.add_item(d, "first")
+        @test key == MathOptInterface.VariableIndex(1)
         @test d[key] == "first"
         @test haskey(d, key) == true
-        @test_throws KeyError d[MyKey(2)]
+        @test_throws KeyError d[MathOptInterface.VariableIndex(2)]
         delete!(d, key)
         @test_throws KeyError d[key]
         @test_throws KeyError d[key] = "key"
         @test haskey(d, key) == false
         key2 = CleverDicts.add_item(d, "second")
-        @test key2 == MyKey(2)
+        @test key2 == MathOptInterface.VariableIndex(2)
         @test d[key2] == "second"
         @test d.vector === nothing
         @test d.dict !== nothing
@@ -32,25 +38,25 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
         empty!(d)
 
         key = CleverDicts.add_item(d, "first")
-        @test key == MyKey(1)
+        @test key == MathOptInterface.VariableIndex(1)
         @test d[key] == "first"
         d[key] = "zeroth"
         @test d[key] == "zeroth"
         @test haskey(d, key) == true
-        @test_throws KeyError d[MyKey(2)]
+        @test_throws KeyError d[MathOptInterface.VariableIndex(2)]
         delete!(d, key)
         @test_throws KeyError d[key]
         @test_throws KeyError d[key] = "key"
         @test haskey(d, key) == false
         key2 = CleverDicts.add_item(d, "second")
-        @test key2 == MyKey(2)
+        @test key2 == MathOptInterface.VariableIndex(2)
         @test d[key2] == "second"
         @test d.vector === nothing
         @test d.dict !== nothing
     end
 
     @testset "LinearIndex" begin
-        d = CleverDicts.CleverDict{MyKey, String}()
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
         key = CleverDicts.add_item(d, "first")
         @test d[CleverDicts.LinearIndex(1)] == "first"
         key2 = CleverDicts.add_item(d, "second")
@@ -65,68 +71,68 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
     end
 
     @testset "keys/values" begin
-        d = CleverDicts.CleverDict{MyKey, String}()
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
         key = CleverDicts.add_item(d, "first")
         key2 = CleverDicts.add_item(d, "second")
-        @test collect(keys(d)) == [MyKey(1), MyKey(2)]
+        @test collect(keys(d)) == [MathOptInterface.VariableIndex(1), MathOptInterface.VariableIndex(2)]
         @test collect(values(d)) == ["first", "second"]
         delete!(d, key)
         key3 = CleverDicts.add_item(d, "third")
-        @test collect(keys(d)) == [MyKey(2), MyKey(3)]
+        @test collect(keys(d)) == [MathOptInterface.VariableIndex(2), MathOptInterface.VariableIndex(3)]
         @test collect(values(d)) == ["second", "third"]
     end
 
     @testset "iterate" begin
-        d = CleverDicts.CleverDict{MyKey, String}()
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
         key = CleverDicts.add_item(d, "first")
         key2 = CleverDicts.add_item(d, "second")
-        my_keys = MyKey[]
+        my_keys = MathOptInterface.VariableIndex[]
         my_values = String[]
         for (k, v) in d
            push!(my_keys, k)
            push!(my_values, v)
         end
-        @test my_keys == [MyKey(1), MyKey(2)]
+        @test my_keys == [MathOptInterface.VariableIndex(1), MathOptInterface.VariableIndex(2)]
         @test my_values == ["first", "second"]
         delete!(d, key)
         key3 = CleverDicts.add_item(d, "third")
-        my_keys = MyKey[]
+        my_keys = MathOptInterface.VariableIndex[]
         my_values = String[]
         for (k, v) in d
            push!(my_keys, k)
            push!(my_values, v)
         end
-        @test my_keys == [MyKey(2), MyKey(3)]
+        @test my_keys == [MathOptInterface.VariableIndex(2), MathOptInterface.VariableIndex(3)]
         @test my_values == ["second", "third"]
     end
 
     @testset "iterate ii" begin
-        d = CleverDicts.CleverDict{MyKey, String}()
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
         key = CleverDicts.add_item(d, "first")
         key2 = CleverDicts.add_item(d, "second")
-        my_keys = MyKey[]
+        my_keys = MathOptInterface.VariableIndex[]
         my_values = String[]
         for (k, v) in d
             push!(my_keys, k)
             push!(my_values, v)
         end
-        @test my_keys == [MyKey(1), MyKey(2)]
+        @test my_keys == [MathOptInterface.VariableIndex(1), MathOptInterface.VariableIndex(2)]
         @test my_values == ["first", "second"]
         delete!(d, key)
         @test d[CleverDicts.LinearIndex(1)] == "second"
         key3 = CleverDicts.add_item(d, "third")
-        my_keys = MyKey[]
+        my_keys = MathOptInterface.VariableIndex[]
         my_values = String[]
         for (k, v) in d
             push!(my_keys, k)
             push!(my_values, v)
         end
-        @test my_keys == [MyKey(2), MyKey(3)]
+        @test my_keys == [MathOptInterface.VariableIndex(2), MathOptInterface.VariableIndex(3)]
         @test my_values == ["second", "third"]
     end
 
     @testset "iterate iii" begin
-        d = CleverDicts.CleverDict{MyKey, String}()
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
         y = 0
         for (k, v) in d
             y += 1
@@ -135,7 +141,7 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
     end
 
     @testset "haskey" begin
-        d = CleverDicts.CleverDict{MyKey, String}()
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
         @test !haskey(d, 1)
         k = CleverDicts.add_item(d, "a")
         @test haskey(d, k)
@@ -147,7 +153,7 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
     end
 
     @testset "haskey" begin
-        d = CleverDicts.CleverDict{MyKey, String}()
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
         @test isempty(d) == true
         k = CleverDicts.add_item(d, "a")
         @test isempty(d) == false
@@ -158,9 +164,9 @@ CleverDicts.index_to_key(::Type{MyKey}, index::Int) = MyKey(index)
     end
 
     @testset "delete!" begin
-        d = CleverDicts.CleverDict{MyKey, String}()
+        d = CleverDicts.CleverDict{MathOptInterface.VariableIndex, String}()
         @test length(d) == 0
-        @test delete!(d, MyKey(0)) == nothing
+        @test delete!(d, MathOptInterface.VariableIndex(0)) == nothing
         k1 = CleverDicts.add_item(d, "a")
         k2 = CleverDicts.add_item(d, "b")
         d[CleverDicts.LinearIndex(2)] == "b"