Spelling of API methods getURL and setUrl of imagelayers is inconsistent and confusing

[marcjansen]

Every instance of OpenLayers.Layer.Image has the following two methods to set/get the URL of the layer:

• getURL
• setUrl

The spelling is inconsequent (Url vs. uppercase-only URL) and therefore rather confusing.

Attached is a patch that renames the method setUrl to setURL and remains a deprecated copy of the method setUrl so that existing applications using the old method don't break.

[marcjansen]

the patch that changes the spelling of the method setUrl to setURL

[bartvde]

Marc, you are absolutely right, but personally I would rather leave this until OpenLayers 3.0. What do others think?

[bartvde]

Not a regression so bumping. I am taking this to the 3.0 milestone since I think it is more logical to change it there and then. Move back to 2.10 if you or some else disagrees.

[pspencer]

for instances like this, would it be useful to add a deprecation utility method, perhaps OpenLayers.Function.deprecated() so that you can do something like: getUrl: function() { /* the original implementation, renamed for consistency */ }, getURL: OpenLayers.Function.deprecated(OpenLayers.Layer.Image.getUrl),

where OpenLayers.Function.deprecated would issue a warning via console or alert depending on some global setting and then call the original function?

[marcjansen]

pspencer, this is more or less what the attached patch does: it keeps the old method and internally calls the new one. It does not, however, issue a dprecated-warning of any kind. The API-documentation for the old method contains a textual warning though.

The patch does not break the API, so I would love to have it in 2.9. As for the next version (2.10 or 3.0) the old (in 2.9 deprecated) and confusing method could be removed IMO.

• 2.8: method there and inconsistent/confusing
• 2.9: method there but calls a more guessable method and marks the confusing one as deprecated
• 2.10/3.0: only the sane method is kept

Just my two cents. If people disagree on this, the version for this can be changed again. I promise to leave the ticket untouched if it is moved to future again :-)

[bartvde]

We do have a mechanism in place to warn for deprecation e.g.:

        var msg = "The OpenLayers.Layer.WMS.Untiled class is deprecated and " +
                  "will be removed in 3.0. Instead, you should use the " +
                  "normal OpenLayers.Layer.WMS class, passing it the option " +
                  "'singleTile' as true.";
        OpenLayers.Console.warn(msg);

Marc, can you update your patch with something like this?

[marcjansen]

The patch that changes the spelling of setUrl to setURL and issues a deprecated-warning when the old method is called.

[bartvde]

(In [10112]) Spelling of API methods getURL and setUrl of imagelayers is inconsistent and confusing, p=marcjansen, r=me (closes #2354)

[bartvde]

Thanks Marc, I am gonna open up a new ticket for 3.0 (ticket:2531) since there is still inconsistency between classes about URL versus Url, e.g. HTTPRequest uses setUrl.

[tschaub]

Sorry I didn't comment on this earlier. I think this change was a bit short-sighted. It would be a change of less consequence to deprecate getURL on the image layer. All other layers that inherit from HTTPRequest get  setUrl.

All other getURL methods on layers are not marked as part of the API. The Image layer is the only layer with a getURL API method. If you want to deprecate something, deprecate that.

[bartvde]

Thanks Tim, right I had noticed the inconsistency with HTTPRequest, but I thought URL would be a better way to write than Url. Are you suggesting to make getURL a non-api method as the only change here and reverting r10112 ?

[tschaub]

Given that the image layer API has setUrl and getURL, and all other HTTPRequest layers have setUrl (and no public getURL), the simplest way to get consistency in the image layer API is to add getUrl and deprecate getURL. Adding setURL to the image layer API makes it inconsistent with every other HTTPRequest layer (which feels like a bigger inconsistency to me).

One alternative is to do nothing. When we do a more thorough review of the API, we can discuss whether we like setURL or setUrl and make a consistent change throughout (adding a new method and deprecating the old if there is sufficient motivation).

Anyway, in order of sensibility (to me), the alternatives are A) to do nothing with the image layer until we do a thorough renaming toward 3.0, B) to add getUrl to the image layer (making its API consistent), or C) to add setURL to the image layer (making it inconsistent with every other HTTPRequest layer.

Alternatives A and B involve reverting r10112.

[bartvde]

Ok thanks for the thorough explanation Tim, I am in favour of A or B (I do not have a real preference here as I said before my initial intention was to leave all this to 3.0, but I can also see the confusion).

Marc do you have enough interest in B) ? If so let me know and I'll change the code.

[marcjansen]

I wouldn't mind if r10112 was reverted. And bumping this up to the future wouldn't hurt my feelings either. I agree that the change in r10112 introduces as much inconsistency as it removes, at least when compaired to layers different from Layer.Image.

To get this issue closed I'd suggest

• reverting r10112
• creating a new ticket concerning the URL- vs. Url-methods throughout the library (setting the milestone to 2.10 or 3.0)
• closing this ticket with a reference to the new one

If someone is really annoyed from the minor inconsistency in Layer.Image that this ticket originally adressed, he/she can probably do the needed changes by hand.

[bartvde]

reverted in r10139

Marc, I had already opened up a new ticket, see ticket:2531 for the future issue. Feel free to add comments there.

Thanks.