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?

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