Help to refactor my code

ruby-talk
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])

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 work
as `File:open` with block.

···

--
Posted via http://www.ruby-forum.com/.

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.

···

On Sun, Feb 9, 2014 at 7:13 AM, Arup Rakshit <lists@ruby-forum.com> wrote:

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])

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 work
as `File:open` with block.

--
Posted via http://www.ruby-forum.com/\.

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
duplicates. But `a.delete_if { |e| e == num_to_del }` will delete all
the elements, which has occurrence 3 or more than 3.

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

···

--
Posted via http://www.ruby-forum.com/\.

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.

···

On Sun, Feb 9, 2014 at 7:47 AM, Arup Rakshit <lists@ruby-forum.com> wrote:

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
duplicates. But `a.delete_if { |e| e == num_to_del }` will delete all
the elements, which has occurrence 3 or more than 3.

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

--
Posted via http://www.ruby-forum.com/\.

5

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, 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.

--
Posted via http://www.ruby-forum.com/\.

6

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.

···

On Sun, Feb 9, 2014 at 8:19 AM, Arup Rakshit <lists@ruby-forum.com> wrote:

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, 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.

--
Posted via http://www.ruby-forum.com/\.

7

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.

Thank you very much. It reads nice to me.

···

--
Posted via http://www.ruby-forum.com/\.

8

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.

···

On Sun, Feb 9, 2014 at 9:38 AM, Arup Rakshit <lists@ruby-forum.com> wrote:

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.

Thank you very much. It reads nice to me.

--
Posted via http://www.ruby-forum.com/\.

9

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.

···

--
Posted via http://www.ruby-forum.com/\.