substancelab/rconomic

Handle is (potentially) not up to date after creating a DebtorContact

Closed this issue · 3 comments

When you create a DebtorContact through the E-Conomic API, it is not possible to either set or predict the the next ID for a contact. This means that a contact is created by setting the ID to 0 as described in lib/economic/debtor_contact.rb, where E-Conomic will assign an ID to the response.

The problem is that the ID of the handle of the DebtorContact is not updated after the DebtorContact has been created. This means that when you try to create an invoice and sets the attention to a DebtorContact that you have just created, the request will fail, because E-Conomic can't find the newly created contact (if the handle ID is 0). This happens if the handle method is called, when the DebtorContact is being build, which will result in the Handle being memoized with an outdated ID.

My suggestion is to stop memoizing in the handle method such that it is defined by:

def handle
  @handle || Handle.build({:id => @id})
end

instead of

def handle
  @handle ||= Handle.build({:id => @id})
end

I see no reasons for memoizing her as there is no real performance gain. And with this suggestion there wil be command-query separation and the handle will always be in synch with the Entity unless it explicitly is being assigned to another handle.

Are there any thoughts on this? Else i will make a PR on it.

I'm guessing the memoization is there to prevent a new Handle object being created whenever #handle is requested. I don't if or how much of an impact this could have.

But certainly, the memoized handle should be reset whenever it needs updating. Check out CurrentInvoice which seems to handle this - both for specs and implementation details.

Yes. I understand why the handle is being memoized, but IMHO memoization should only be used, when there is a gain to it (e.g. performance). And performance is not a reason here. Else it just adds unnecessary complexity. If we remove this memoization, we would actually be able to remove all these resets that we have to add to make the system work as we expect.