Looping and conditional deleting array elements

The following simple example works

x = [1,2,3,4]
(0..x.size()-1).each do |i|

  if i == 1 || i == 2
    x.delete_at(i)
  end
end
puts x

The next example errors

x = [1,2,3,4]
(0..x.size()-1).each do |i|

  val = x[i]
  if i == 1 || i == 2
    x.delete_at(i)
  end
end
puts x
Unhandled exception: Index out of bounds (IndexError)
  from /usr/share/crystal/src/indexable.cr:614:8 in '[]'
  from test1.cr:4:3 in '__crystal_main'
  from /usr/share/crystal/src/crystal/main.cr:97:5 in 'main_user_code'
  from /usr/share/crystal/src/crystal/main.cr:86:7 in 'main'
  from /usr/share/crystal/src/crystal/main.cr:106:3 in 'main'
  from __libc_start_main
  from _start
  from ???

The next example works

x = [1,2,3,4]
(0..x.size()-1).each do |i|

  val = 0
  begin
    val = x[i]
  rescue
  end
  if i == 1 || i == 2
    x.delete_at(i)
  end
end
puts x

Question : is this the correct way of deleting array elements where I use the begin rescue end clause ?

In this example I am using integers. The real use case is, is that I am looping
over nodes where I need to determine the type of node
and based on its type delete the node or not.

The usual way is using Array#reject!. Also try using #reverse_each instead of #each in your example.

Why is (0..x.size()-1) being used instead of just x?

I don’t think I can use your solution, as I need to access each array element (determine its class and/or properties). I suppose I gave the wrong example, here a more realistic example

class Node
 ...
end

class Node1 < Node
 ...
end

def get_nodes() : Array(Node)
  ...
end

x = get_nodes()
(x.size()-1).downto(0).each do |i|
  
  node = x[i]
  case node.class.to_s
    when "Node1"

    when "Node2"
      # do something special
     x.delete_at(i)

    # more node types
  end
end
x = [1,2,3,4]
x.each_with_index do |v, i|

  if i == 1 || i == 2
  pp "Trying to delete #{i}.."
    x.delete_at(i)
  end
end


puts x

@gummybears try this!

delete_at deletes using the index, not the value. You were passing in the value of the array element, not the index ;)

With reject! it would be like:

x = get_nodes()
x.reject! do |node|
  case node.class.to_s
  when "Node1"
    false # this node won't get deleted
  when "Node2"
    # do something special
    true # this node will get deleted

  # and so on
  end
end

Thanks for all the help, I am going to try each of your solutions.

It will use less memory and be much faster if to do class comparison this way (it compares with type_id and not allocates any memory for strings and not doing string comparison):

    x = get_nodes()
    x.reject! do |node|
      case node
      when Node1
        false # this node won't get deleted
      when Node2
        # do something special
        true # this node will get deleted

      # and so on
      end
    end

or even better:

    x = get_nodes()
    x.reject! do |node|
      case node
      when Node2 # you can add more node types here like this Node2, Node3, Node5 and so on
        # do something special
        true # this node will get deleted
      else
        false # this node won't get deleted
      end
    end
2 Likes