Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Proposal: change getFromFirst to return non-optional #5484

Closed
CeylonMigrationBot opened this issue Dec 15, 2014 · 4 comments
Closed

Proposal: change getFromFirst to return non-optional #5484

CeylonMigrationBot opened this issue Dec 15, 2014 · 4 comments

Comments

@CeylonMigrationBot
Copy link

[@jvasileff] I wasn't sure how to make this proposal, since at the face of it, it seems un-Ceylon like. But I think it is worth consideration.

Currently, Array.getFromFirst(index) returns Element?. My contention, however, is that the normal use case is for it to be immediately followed by an assertion that the return value exists (when Element satisfies Object, of course). And, therefore, getFromFirst and getFromLast should return Element, with an AssertionError for invalid indexes.

Unlike keys for maps and sets, array indexes are not usually identifiers for the stored elements; their use as identifiers is usually relevant only in terms of the array itself. So, when attempting array.getFromFirst(10), you probably expect the call to succeed. The value 10 is likely to have come from something like a variable used for iteration, or maybe the result of a find method, rather than something unrelated to the structure of the list, like an employee id.

The properties first and last are of course very different, and will often be used without the knowledge of whether the array is empty or not.

So I think there are a few reasons and justifications for this change:

  • Optimize for the normal use case; eliminate unnecessary assertions
  • Make List.getFromFirst symmetric with ListMutator.set in terms of throwing an AssertionError on index out of bounds.
  • The get(Key) method would still be available, for when Element? is preferred.
  • I suspect there would be minimal impact to existing code. Idiomatic Ceylon avoids explicit access by index except in special circumstances.
  • Incidentally, but importantly, I believe this change may be necessary for potential support of specialized get methods for Array<primitive>s.

In an attempt to be objective, a few counter points:

  • Returning Element? may be of use when processing corresponding elements of two or more arrays of different sizes, where default values may be used for missing elements.
  • I suppose you may know in advance that an array should store exactly 100 elements, but as an optimization, you may wish to start with a smaller array and expand it if necessary, and for whatever reason, rely on an exists test on the return value of getFromFirst to determine the current size. But, really, there are better ways to accomplish this.
  • I have been focussing on Array, but getFromFirst is actually defined by Iterable, where arguably the current behavior is more valuable since checking the size of Iterables can be expensive. I'm not sure what the use case is, but if it's important, perhaps getFromFirstElse(index, fallback) could be defined. Or, just use iterable.skip(index).first.

[Migrated from ceylon/ceylon.language#606]

@CeylonMigrationBot
Copy link
Author

[@quintesse] The other idea was, I think, to add a different method for that, something like unsafeGet() and/or unsafeGetFromFirst(). But if this issue is specifically meant for Array maybe my proposal #5490 to extend the idea we use for Sequences when dealing with emptiness would help here as well?

@jvasileff
Copy link
Contributor

The other idea was, I think, to add a different method for that, something like unsafeGet() and/or unsafeGetFromFirst()

That was discussed for Arrays, as a purely performance related concern. But for this issue, I'm going beyond performance, and claiming that a non-optional return for getFromFirst is simply better.

Optional returns provide two potential advantages:

  1. A reminder to the developer that there may be no element at the given index, and
  2. Language conveniences (such as else) for dealing with the OOB case

But since it's unusual to attempt an access-by-index without already knowing the index is valid, being forced to deal with the possibility that it is invalid is an annoyance, and can even lead to hard to find bugs when shortcuts are taken like list.getFromFirst(knownGoodIndexButReallyOffByOne) else 0.

@jvasileff
Copy link
Contributor

Seems this should be accepted or closed for the next release.

@jvasileff
Copy link
Contributor

This issue, which I know is a tough sell anyway, has been open for a couple years already without a word from Gavin. So, closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants