Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map.get() should throw if there is no value, and we should also offer tryGet() #3575

Closed
eladb opened this issue Jul 23, 2023 · 7 comments · Fixed by #4523
Closed

Map.get() should throw if there is no value, and we should also offer tryGet() #3575

eladb opened this issue Jul 23, 2023 · 7 comments · Fixed by #4523
Assignees
Labels
🐛 bug Something isn't working good first issue Good for newcomers 🎨 sdk SDK

Comments

@eladb
Copy link
Contributor

eladb commented Jul 23, 2023

I tried this:

let m = { "foo" => "bar" };
let s: str = m.get("zoo");
assert(s.length > 0);

This happened:

ERROR: Cannot read properties of undefined (reading 'length')

I expected this:

I expected this to throw.

I also expected to find tryGet() to return T?.

Is there a workaround?

If we assign the result to a T?, it works.

let m = { "foo" => "bar" };
let s: str? = m.get("zoo");
assert(s == nil);

Component

SDK

Wing Version

No response

Node.js Version

No response

Platform(s)

No response

Anything else?

No response

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
@eladb eladb added the 🐛 bug Something isn't working label Jul 23, 2023
@eladb eladb changed the title Map.get should throw if there is no value, and we should also offer tryGet() Map.get() should throw if there is no value, and we should also offer tryGet() Jul 23, 2023
@Chriscbr Chriscbr added the 🎨 sdk SDK label Jul 23, 2023
@staycoolcall911 staycoolcall911 added the good first issue Good for newcomers label Jul 25, 2023
@giovanni-orciuolo
Copy link

Hello! Can I work on this issue? I want to get my hands dirty with the codebase and this seems like a good starting point.

@ekeren
Copy link

ekeren commented Jul 31, 2023

Yes, it does make a lot of sense to start with this

@ekeren
Copy link

ekeren commented Jul 31, 2023

I've assigned it to you @giovanni-orciuolo , let me know if you need any help with this

@revitalbarletz
Copy link
Contributor

Hi @giovanni-orciuolo ! How about joining our slack channel? https://t.winglang.io/slack

mergify bot pushed a commit that referenced this issue Aug 8, 2023
There are several open issues in progress that need to be able to define generic optionals. 
Such as: 
- #1868
- #3575
- #3653

I only added `tryAt` method for Array since the actual work is defined in another issue, but I needed an example to write some test cases with. Since there are several efforts that need this change I opted to not wait and include it in one of those PRs but just add the support here so other efforts can rebase.

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@github-actions
Copy link

github-actions bot commented Oct 4, 2023

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@github-actions github-actions bot added the Stale label Oct 4, 2023
@staycoolcall911
Copy link
Contributor

Keep

@github-actions github-actions bot removed the Stale label Oct 5, 2023
@mergify mergify bot closed this as completed in #4523 Oct 12, 2023
mergify bot pushed a commit that referenced this issue Oct 12, 2023
Includes the changes from: #3771 that giovanni did.

Closes: #3575

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.

Co-authored-by: Giovanni Orciuolo <giovanni@orciuolo.it>
BREAKING CHANGE: `Map.get()` now throws if key does not exist.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.38.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working good first issue Good for newcomers 🎨 sdk SDK
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants