Reimplement `Hash#compact!` to conform the standard behavior.

`Hash#compact!` should return `nil` if the receiver does not change.

2 files changed, 36 insertions, 50 deletions

diff --git a/mrbgems/mruby-hash-ext/mrblib/hash.rb b/mrbgems/mruby-hash-ext/mrblib/hash.rb
index 549bca0a8..eaf54b871 100644
--- a/mrbgems/mruby-hash-ext/mrblib/hash.rb
+++ b/mrbgems/mruby-hash-ext/mrblib/hash.rb
@@ -116,6 +116,31 @@ class Hash
 
   ##
   # call-seq:
+  #   hsh.compact!    -> hsh
+  #
+  # Removes all nil values from the hash. Returns the hash.
+  # Returns nil if the hash does not contain nil values.
+  #
+  #   h = { a: 1, b: false, c: nil }
+  #   h.compact!     #=> { a: 1, b: false }
+  #
+
+  def compact!
+    h = {}
+    keys = self.keys
+    nk = keys.select{|k|
+      self[k] != nil
+    }
+    return nil if (keys.size == nk.size)
+    nk.each {|k|
+      h[k] = self[k]
+    }
+    h
+    self.replace(h)
+  end
+
+  ##
+  # call-seq:
   #    hsh.compact     -> new_hsh
   #
   # Returns a new hash with the nil values/key pairs removed
@@ -125,9 +150,13 @@ class Hash
   #    h             #=> { a: 1, b: false, c: nil }
   #
   def compact
-    result = self.dup
-    result.compact!
-    result
+    h = {}
+    self.keys.select{|k|
+      self[k] != nil
+    }.each {|k|
+      h[k] = self[k]
+    }
+    h
   end
 
   ##
diff --git a/mrbgems/mruby-hash-ext/src/hash-ext.c b/mrbgems/mruby-hash-ext/src/hash-ext.c
index 6619f5268..e6112667b 100644
--- a/mrbgems/mruby-hash-ext/src/hash-ext.c
+++ b/mrbgems/mruby-hash-ext/src/hash-ext.c
@@ -37,42 +37,6 @@ hash_values_at(mrb_state *mrb, mrb_value hash)
 }
 
 /*
- * call-seq:
- *   hsh.compact!    -> hsh
- *
- * Removes all nil values from the hash. Returns the hash.
- *
- *   h = { a: 1, b: false, c: nil }
- *   h.compact!     #=> { a: 1, b: false }
- */
-static mrb_value
-hash_compact_bang(mrb_state *mrb, mrb_value hash)
-{
-  khiter_t k;
-  khash_t(ht) *h = RHASH_TBL(hash);
-  mrb_int n = -1;
-
-  if (!h) return mrb_nil_value();
-  for (k = kh_begin(h); k != kh_end(h); k++) {
-    if (kh_exist(h, k)) {
-      mrb_value val = kh_value(h, k).v;
-      khiter_t k2;
-
-      if (mrb_nil_p(val)) {
-        kh_del(ht, mrb, h, k);
-        n = kh_value(h, k).n;
-        for (k2 = kh_begin(h); k2 != kh_end(h); k2++) {
-          if (!kh_exist(h, k2)) continue;
-          if (kh_value(h, k2).n > n) kh_value(h, k2).n--;
-        }
-      }
-    }
-  }
-  if (n < 0) return mrb_nil_value();
-  return hash;
-}
-
-/*
  *  call-seq:
  *     hsh.slice(*keys) -> a_hash
  *
@@ -85,28 +49,22 @@ hash_compact_bang(mrb_state *mrb, mrb_value hash)
 static mrb_value
 hash_slice(mrb_state *mrb, mrb_value hash)
 {
-  khash_t(ht) *h = RHASH_TBL(hash);
   mrb_value *argv, result;
   mrb_int argc, i;
-  khiter_t k;
-  int ai;
 
   mrb_get_args(mrb, "*", &argv, &argc);
-  if (argc == 0 || h == NULL) {
+  if (argc == 0) {
     return mrb_hash_new_capa(mrb, argc);
   }
   result = mrb_hash_new_capa(mrb, argc);
-  ai = mrb_gc_arena_save(mrb);
   for (i = 0; i < argc; i++) {
     mrb_value key = argv[i];
+    mrb_value val;
 
-    k = kh_get(ht, mrb, h, key);
-    if (k != kh_end(h)) {
-      mrb_value val = kh_value(h, k).v;
-
+    val = mrb_hash_fetch(mrb, hash, key, mrb_undef_value());
+    if (!mrb_undef_p(val)) {
       mrb_hash_set(mrb, result, key, val);
     }
-    mrb_gc_arena_restore(mrb, ai);
   }
   return result;
 }
@@ -118,7 +76,6 @@ mrb_mruby_hash_ext_gem_init(mrb_state *mrb)
 
   h = mrb->hash_class;
   mrb_define_method(mrb, h, "values_at", hash_values_at, MRB_ARGS_ANY());
-  mrb_define_method(mrb, h, "compact!",  hash_compact_bang, MRB_ARGS_NONE());
   mrb_define_method(mrb, h, "slice",     hash_slice, MRB_ARGS_ANY());
 }