# Help to refactor my code

**URL:** <https://www.ruby-forum.com/t/help-to-refactor-my-code/239042>\
**Category:** Ruby\
**Created:** [February 9, 2014, 11:13am UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042 "2014-02-09T11:13:10Z")\
**Posts on this page:** 9\
**Page:** 1

<div class="post-metadata">

**Author:** ![Detlef\_R](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/58f4c7/32.png) [@Detlef\_R](https://www.ruby-forum.com/u/Detlef_R)\
**Post date:** [February 9, 2014, 11:13am UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/1 "2014-02-09T11:13:10Z")

</div>

I want to delete the first 3 duplicate entries -

I wrote a code :

def del\_first\_three(a)  
num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
return a if num\_to\_del.nil?  
3.times do  
ind = a.index { |e| e == num\_to\_del }  
a.delete\_at(ind)  
end  
a  
end

del\_first\_three([3,4,5,3,3]) # =\> [4,5]

But at the method end I put `a`, to return the resultant array, which I  
don’t like. So I took the help of `#tap` as below :

def del\_first\_three(a)  
num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
return a if num\_to\_del.nil?  
3.times do  
ind = a.index { |e| e == num\_to\_del }  
a.tap { |ob| ob.delete\_at(ind) }  
end  
end

del\_first\_three([3,4,5,3,3]) # =\> 3

But it is also helpless, as `Integer#times` returns `self`. Is there any  
method to meet my need. I am actually looking for a method, which will  
be working here as `File::open` with block.

---

<div class="post-metadata">

**Author:** ![my-ruby](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/e95f7d/32.png) [@my-ruby](https://www.ruby-forum.com/u/my-ruby)\
**Post date:** [February 9, 2014, 11:36am UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/2 "2014-02-09T11:36:33Z")

</div>

Dear Arup,

Your code:

def del\_first\_three(a)  
num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
return a if num\_to\_del.nil?  
3.times do  
ind = a.index { |e| e == num\_to\_del }  
a.delete\_at(ind)  
end  
a  
end

First step:

def del\_first\_three(a)  
num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
return a if num\_to\_del.nil?

a.delete\_if { |e| e == num\_to\_del }  
end

# All the part from 3.times do block on got substituted.

# Now is easy to figure out that the first return is also redundant

def del\_first\_three(a)  
num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
a.delete\_if { |e| e == num\_to\_del }  
end

I hope it helps.

Best regards,  
Abinoam Jr.

---

<div class="post-metadata">

**Author:** ![my-ruby](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/e95f7d/32.png) [@my-ruby](https://www.ruby-forum.com/u/my-ruby)\
**Post date:** [February 9, 2014, 11:47am UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/3 "2014-02-09T11:47:20Z")

</div>

Abinoam Jr. wrote in post #1136118:

> Dear Arup,

> # All the part from 3.times do block on got substituted.
> 
> # Now is easy to figure out that the first return is also redundant
> 
> def del\_first\_three(a)  
> num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
> a.delete\_if { |e| e == num\_to\_del }  
> end

Nice re-factoring. But the point is, I want to delete first 3  
occurrence. But `a.delete_if { |e| e == num_to_del }` will delete all  
the occurrence of the element, which has occurrence 3 or more than 3.

say I have [1,3,3,4,7,3,2,3], my expected output will be [1,4,7,2,3]

My point is if any element occurs 4 times, I would delete only first 3  
occurrence.

---

<div class="post-metadata">

**Author:** ![my-ruby](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/e95f7d/32.png) [@my-ruby](https://www.ruby-forum.com/u/my-ruby)\
**Post date:** [February 9, 2014, 12:10pm UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/4 "2014-02-09T12:10:33Z")

</div>

Dear Arup,

Sorry if I misunderstood the requirements.  
But, I think there is two “FIRST THREE” in play over here.

For [3, 4, 5, 3, 3, 4, 4, 4, 5] what do you expect to get in return?

Because the line

num\_to\_del = a.find { |e| a.count(e) \>= 3 }

Finds the only the first number in array that has three duplicates.  
So it will find 3 and not 4. ( I misunderstood this as being a “First  
Three Duplicate Number” )

After that, do you want do delete the first 3 occurrences of the number  
3 ?  
So, if there’s only 3 number 3s. It will rest none of them?  
But, with the number five that has only 2 elements. it will rest the  
two of them? (more than the 3 number)  
And the 4 number, that has more than three duplicates, it rest all the  
four of them so it was not found before?  
Or it will rest 1 element of them, so that it has 4 and 4-3 = 1?  
The sorting order of the elements are important?  
If you have one more triplet like in [3, 4, 5, 3, 3, 3, 3, 3] with 6  
number 3s?  
Do you want the second triplet to be deleted also?

Could you state more clearly your requirements?

Perhaps I could help you (or not 😉 )

Best regards,  
Abinoam Jr.

---

<div class="post-metadata">

**Author:** ![my-ruby](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/e95f7d/32.png) [@my-ruby](https://www.ruby-forum.com/u/my-ruby)\
**Post date:** [February 9, 2014, 1:26pm UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/5 "2014-02-09T13:26:08Z")

</div>

Dear Arup,

I don’t know if the code bellow could read better in your opinion.

But, from your first example you could pass “a” as an object on the  
iteration.

def del\_first\_three\_refactored\_2(a)  
num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
return a if num\_to\_del.nil?  
3.times.with\_object(a) do |turn, obj|  
ind = obj.index { |e| e == num\_to\_del }  
obj.delete\_at(ind)  
end  
end

Does it fit?

And, I could come up with another approach that is faster than your  
first attempts.  
I just don’t know if it will read well for your taste.

def del\_first\_three\_refactored(ary)  
el\_indices = Hash.new { |hash, key| hash[key] = Array.new }  
ary.each\_with\_index do |el, ix|  
el\_indices[el].push ix  
if el\_indices[el].size \>= 3  
el\_indices[el].each\_with\_index do |del\_ix, offset|  
ary.delete\_at(del\_ix - offset)  
end  
break ary  
end  
end  
end

For an array with more than 100\_000 elements

Original (1) lasted 4.272368536 seconds  
Original (2) lasted 4.282821329 seconds  
Refactored lasted 0.618384996 seconds

Best regards,  
Abinoam Jr.

---

<div class="post-metadata">

**Author:** ![my-ruby](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/e95f7d/32.png) [@my-ruby](https://www.ruby-forum.com/u/my-ruby)\
**Post date:** [February 9, 2014, 1:38pm UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/6 "2014-02-09T13:38:55Z")

</div>

Abinoam Jr. wrote in post #1136125:

> Dear Arup,
> 
> I don’t know if the code bellow could read better in your opinion.
> 
> But, from your first example you could pass “a” as an object on the  
> iteration.
> 
> def del\_first\_three\_refactored\_2(a)  
> num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
> return a if num\_to\_del.nil?  
> 3.times.with\_object(a) do |turn, obj|  
> ind = obj.index { |e| e == num\_to\_del }  
> obj.delete\_at(ind)  
> end  
> end
> 
> Does it fit?

This is good! Actually I forgot that `#times` give us `Enumerator`  
object without block.

Thank you very much. It reads nice to me.

---

<div class="post-metadata">

**Author:** ![my-ruby](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/e95f7d/32.png) [@my-ruby](https://www.ruby-forum.com/u/my-ruby)\
**Post date:** [February 9, 2014, 12:19pm UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/7 "2014-02-09T12:19:35Z")

</div>

Abinoam Jr. wrote in post #1136120:

> Dear Arup,
> 
> Sorry if I misunderstood the requirements.  
> But, I think there is two “FIRST THREE” in play over here.
> 
> For [3, 4, 5, 3, 3, 4, 4, 4, 5] what do you expect to get in return?
> 
> Because the line
> 
> num\_to\_del = a.find { |e| a.count(e) \>= 3 }
> 
> Finds the only the first number in array that has three duplicates.  
> So it will find 3 and not 4. ( I misunderstood this as being a “First  
> Three Duplicate Number” )

Yes with your example target element would be 3.

> After that, do you want do delete the first 3 occurrences of the number  
> 3 ?

Yes, exactly. output should be [4, 5, 4, 4, 4, 5]

> So, if there’s only 3 number 3s. It will rest none of them?  
> But, with the number five that has only 2 elements. it will rest the  
> two of them? (more than the 3 number)

If there is 5 3’s, first 3 occurrence will be deleted, rest will be as  
it is.

> And the 4 number, that has more than three duplicates, it rest all the  
> four of them so it was not found before?

Nothing to be done with 4, as we got first 3, which has met the  
condition  
`>=3`

> The sorting order of the elements are important?

elements order should be preserved there, no need to sort.

> If you have one more triplet like in [3, 4, 5, 3, 3, 3, 3, 3] with 6  
> number 3s?

Expected output is [4, 5, 3, 3, 3].

> Best regards,  
> Abinoam Jr.

---

<div class="post-metadata">

**Author:** ![my-ruby](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/e95f7d/32.png) [@my-ruby](https://www.ruby-forum.com/u/my-ruby)\
**Post date:** [February 9, 2014, 2:39pm UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/8 "2014-02-09T14:39:20Z")

</div>

Abinoam Jr. wrote in post #1136127:

> Great and you’re welcome!
> 
> One more thing.  
> I forgot to pass the url of a gist created to benchmark the 3 approachs.
> 
> [In response to https://www.ruby-forum.com/topic/4422661 · GitHub](https://gist.github.com/abinoam/8898249)
> 
> Abinoam Jr.

FYI - I wrote the below code, but it wouldn’t give the desired result as  
I mentioned in main post.

def del\_first\_three(a)  
num\_to\_del = a.find { |e| a.count(e) \>= 3 }  
return a if num\_to\_del.nil?  
3.times do  
ind = a.index { |e| e == num\_to\_del }  
a.tap { |ob| ob.delete\_at(ind) }  
end  
end

del\_first\_three([3,4,5,3,3]) # =\> 3 # \<~~~ see the output,due to #times.

So don’t consider this.

---

<div class="post-metadata">

**Author:** ![my-ruby](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/m/e95f7d/32.png) [@my-ruby](https://www.ruby-forum.com/u/my-ruby)\
**Post date:** [February 9, 2014, 2:31pm UTC](https://www.ruby-forum.com/t/help-to-refactor-my-code/239042/9 "2014-02-09T14:31:09Z")

</div>

Great and you’re welcome!

One more thing.  
I forgot to pass the url of a gist created to benchmark the 3 approachs.

> <https://gist.github.com/abinoam/8898249>

Abinoam Jr.