Help to refactor my code

Ruby
1

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.

2

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.

3

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.

4

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 :wink: )

Best regards,
Abinoam Jr.

5

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.

6

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.

7

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.

8

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

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.

9

Great and you’re welcome!

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

Abinoam Jr.