1000831 - Prefer using insert()/update() than add()/put() for the DataStore interface.

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Description

[Gene Lian (I already quit Mozilla)]

The insert()/update() has a more explicit naming for its purpose and has a more clear error path (failing to call insert() means failed to insert; the same for update()). Mozilla's put()/add() is ambiguous because it can even call add() to update an existing record.

Hope to align this to the W3C version at: http://airpingu.github.io/data-store-api/index.html

Comment 1

[Andrea Marchesini [:baku]]

I think insert()/update() are better names. but if we change them, we break apps.
Ehsan, if we change them and then we write an email to platform and b2g mailing list, is it enough?

Comment 2

[Andrea Marchesini [:baku]]

Attachment: update.patch

Comment 3

[(no longer active)]

I was not around when the API was being designed for the first time but to the best of my knowledge, these names were chosen for consistency with IndexedDB's IDBObjectStore methods, and I think that's a good argument in favor of the existing names.  The fact that the DataStore add function can be used to update the values is definitely odd and inconsistent with IDBObjectStore, so I would be more open to changing that.

Ben, Jonas, what do you think?

As far as when and how to make the change if we decide that we should do it at all, firstly I think we need to figure out exactly what things we want to change in the API, announce our plans in advance on dev-webapi and dev-gaia and then make all of the changes we want to make at once, with prior coordination with the Gaia teams that are using the API right now.  We should also make sure to make any changes that we want to make before we open up the API to third-party applications (which as far as I understand is a 2.0 goal.)  Given the fact that there are other problems with the API too, we should probably hold off here for now.

Comment 4

[(no longer active)]

Comment on attachment 8411786 [details] [diff] [review]
update.patch

Clearing the review request while we decide whether we want to make this change.

Comment 5

[Gene Lian (I already quit Mozilla)]

(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #3)
> As far as when and how to make the change if we decide that we should do it
> at all, firstly I think we need to figure out exactly what things we want to
> change in the API, announce our plans in advance on dev-webapi and dev-gaia
> and then make all of the changes we want to make at once, with prior
> coordination with the Gaia teams that are using the API right now.  We
> should also make sure to make any changes that we want to make before we
> open up the API to third-party applications (which as far as I understand is
> a 2.0 goal.)  Given the fact that there are other problems with the API too,
> we should probably hold off here for now.

Fortunately, insert()/update() are totally new name and DataStore API is not yet exposed to public (third-party apps) so we won't face too much pain about incompatibility issue. We may do:

1. A Gecko fix to add insert()/update() but still keep add()/put().
2. A Gaia fix to change apps to use insert()/update().
3. A Gecko fix to remove add()/put().

And of course, I'd like to hear from Jonas and Bent's opinions first about if this change makes sense.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't feel strongly. However generally developers prefer short names. And alining with the IndexedDB standard seems nice.

But of course .add() should not work if an object with the provided id already exists.

Comment 7

[(no longer active)]

(In reply to comment #6)
> But of course .add() should not work if an object with the provided id already
> exists.

I think this is basically a separate issue which needs to be filed and fixed no matter what naming we end up with here.

I honestly don't see a good case for changing these names here, I don't think either of the two alternatives are clear winners in terms of clarity.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I agree with comment 7.

Comment 9

[Gene Lian (I already quit Mozilla)]

Tentatively close this one as WONTFIX, which is not worth renaming the names.