DYN-6985 Get item from list with negative index (Wishlist #187)

[tinrobot2000]

Purpose

Update to List.GetItemAtIndex to allow negative index (get from right to left) retrieval
DynamoDS/DynamoWishlist#187

Declarations

Check these if you believe they are true

• The codebase is in a better state after this PR
• Is documented according to the standards
• The level of testing this PR includes is appropriate
• User facing strings, if any, are extracted into *.resx files
• All tests pass using the self-service CI.
• Snapshot of UI changes, if any.
• Changes to the API follow Semantic Versioning and are documented in the API Changes document.
• This PR modifies some build requirements and the readme is updated
• This PR contains no files larger than 50 MB

Release Notes

Update to List.GetItemAtIndex to allow negative index (get from right to left) retrieval

Reviewers

@DynamoDS/dynamo

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

[github-actions]

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

[QilongTang]

hi @tinrobot2000 Thanks a lot for proposing this PR, my team will review and make a decision soon on this. Good work on addressing wishlist issue!

[reddyashish]

@aparajit-pratap @mjkkirschner Do you think this is as per design to not gte the negative index from the list items?

[mjkkirschner]

@aparajit-pratap @mjkkirschner Do you think this is as per design to not gte the negative index from the list items?

If you checkout the builtin implementation of Get indexing it does support negative indices:

Dynamo/src/Libraries/DesignScriptBuiltin/Builtin.cs

Line 33 in 49a3f7c

public static object ValueAtIndex(IList list, int index)

I think this would be an okay change but it requires the following changes IMO.

1. a test
2. consider performance impact, 1 if statement probably is not too bad.
3. I don't see any reason to raise the IndexOutOfRangeException, the indexing operation is going to do that anyway, what am I missing?

[github-actions]

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-6985

[QilongTang]

@aparajit-pratap @mjkkirschner Do you think this is as per design to not gte the negative index from the list items?

If you checkout the builtin implementation of Get indexing it does support negative indices:

Dynamo/src/Libraries/DesignScriptBuiltin/Builtin.cs

Line 33 in 49a3f7c

public static object ValueAtIndex(IList list, int index)

I think this would be an okay change but it requires the following changes IMO.

1. a test
2. consider performance impact, 1 if statement probably is not too bad.
3. I don't see any reason to raise the IndexOutOfRangeException, the indexing operation is going to do that anyway, what am I missing?

Hi @mjkkirschner I think when index is too large or too negative to handle, it could be out of range so exception is only needed in that case. I am adding unit tests for those cases as well.

[mjkkirschner]

Hi @mjkkirschner I think when index is too large or too negative to handle, it could be out of range so exception is only needed in that case. I am adding unit tests for those cases as well.

yeah I'm still not getting it - when you do

var x= int[1]
var y = x[10000000]

that will throw an out of range exception, you don't have to detect it yourself first...