Small refactor and a bit clean up

[khezam]

The logic of ==(other) method was a bit confusing so I tried to clean it up to make it easy to read. I also moved new_from_response class method of Data into class << self for consistency.

Let me know if there is anything I can change and make it better.

[ecoologic]

Thank you @khezam for your contribution ❤️

I might agree in principle on the == changes, but I'm concerned about unexpected changes for all existing clients.

This PR also lacks testing, but don't worry about test/spec live just yet, at this point they are expected to fail, we'll run them if we decide to proceed.

cc @snatchblock, what do you think of these changes?

[khezam]

My pleasure @ecoologic :)

I see your concerns about unexpected changes for all existing clients and the lack of testing. I was/am expecting the existing tests would catch any unexpected behaviors since this PR only cleans up the current logic of ==.

I'm happy to add a test coverage if needed.

Let me know :)

[khezam]

Hey @ecoologic, any update on this PR?

[snatchblock]

Hi Khezam,

Thank you for the cleanup. Can you add test cases for the code to ensure these changes does not impact the library behaviour? Specifically if objects that before didn’t match with equality might start matching.

Thank you. We will merge this once the test cases and validation is done.

[khezam]

Hi @snatchblock,

I definitely can add test cases, but this PR does not need test cases because it does not add/remove from the existing logic, it's only rearranging the existing logic. The test cases already exist here, and if I add test cases, they'll be exactly the same.

The two methods below are exactly the same but the only difference is that the top one is easy to read.

def ==(other)
    return false unless other

    return true if other.object_id == object_id

    return other.id && (other.id == id) if other.is_a?(Data)

    return id == other if other.is_a?(Integer)

    warn "Trying to compare #{other.class} to a Resource
      from #{caller.first}"
 end

  def ==(other)
      return true if other.object_id == object_id

      if other && !(other.is_a?(Data) || other.is_a?(Integer))
        warn "Trying to compare #{other.class} to a Resource from #{caller.first}"
      end

     if other.is_a?(Data)
        other.id && other.id == id
     elsif other.is_a?(Integer)
        id == other
     else
        false
     end
  end

Let me know if I'm missing anything and I'm happy to add test cases accordingly.