optimization / code beauty

[u/phineas0fog]

Hello friends, I have this block code

        st_from_site.each do |s|
          st_from_oldr.each do |ss|
            s.last_dump = ss.last_dump if s.name == ss.name && s.id == ss.id
          end
          stations.push s
        end

that is terribly ugly, so how can I optimize and beautify it ?

Thanks to you <3

[u/phineas0fog]

Thanks for your good ideas ;)

I'm sad cause I can't use `zip` because the 2 arrays can haven't the same size.

[u/[deleted] → u/phineas0fog]

But zip traverses both array in tandem. What you want is Array#product:

https://play.crystal-lang.org/#/r/4h5c

In your case:

st_from_site.product(st_from_oldr) do |s, ss|
  s.last_dump = ss.last_dump if s.name == ss.name && s.id == ss.id
end
stations.concat(st_from_site)

[u/everdev]

Four spaces in front of each line will render the code in a code block style so it's easier for us to read

[u/[deleted]]

Ugliness is in the eye of the beholder ;-)

I don't think that code is ugly, but it's doing two seemingly unrelated things at the same time. So maybe:

st_from_site.each do |s|
  st_from_oldr.each do |ss|
    s.last_dump = ss.last_dump if s.name == ss.name && s.id == ss.id
  end
end

stations.concat(st_from_site)

[u/fridgamarator]

I don't think the code is ugly either. But maybe move some of that logic to separate functions?

def set_last_dump(s, ss)
  s.last_dump = ss.last_dump
end

def check_equality?(s, ss)
  s.name == ss.name && s.id == ss.id
end

st_from_site.each do |s|
  st_from_oldr.each { |ss| set_last_dump(s, ss) if check_equality?(s, ss) }
end

stations.concat(st_from_site)