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

Updating method signature in flatten array #2900

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

jagdish-15
Copy link
Contributor

Pull Request

This PR addresses issue #2896 by updating the method signature in the starter code. Additionally, it updates the example solution to align with the revised method signature in the starter code.


Reviewer Resources:

Track Policies

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jagdish-15, I've just had a look and think this looks good. I just need a bit of time to see if this can work with List<?> flatten(List<?> list) instead of List<Object> flatten(List<?> list).

@jagdish-15
Copy link
Contributor Author

Hey @kahgoh, thanks for taking a look. I totally get that you want to check if List<?> flatten(List<?> list) can work instead of List<Object> flatten(List<?> list). From what I saw, the wildcard type (?) was causing some issues with the assertions.

Feel free to take your time, and let me know if you need anything from my side! Looking forward to hearing your thoughts.

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jagdish-15, after spending time looking at trying to change to the List<?> flatten(List<?>) I think we should stick with what you have for now.

The only way (to change it) I could think of is to change the assertion to isEqualTo(expected) list. The downside with this is that isEqualTo doesn't provide as much information as containsExactly (containsExactly can tell you what it couldn't find or what it didn't expect, whereas isEqualTo will only tell you the lists aren't equal). I like the extra information from the containsExactly.

I'll approve and merge this one.

@kahgoh kahgoh merged commit 406af2a into exercism:main Jan 14, 2025
4 checks passed
@jagdish-15
Copy link
Contributor Author

@kahgoh, I noticed that the config.json file for this exercise lists 21 contributors, but only 17 are visible on the website. Initially, I thought this issue affected only me, but I now realize it’s consistent. What could be causing this discrepancy?

@kahgoh
Copy link
Member

kahgoh commented Jan 15, 2025

Recently, there was a problem with the Github actions that the CI relies on. I had noticed they failed after I merged it. I was going to see if this gets fixed with the next run when the next PR is merged.

1 similar comment
@kahgoh
Copy link
Member

kahgoh commented Jan 15, 2025

Recently, there was a problem with the Github actions that the CI relies on. I had noticed they failed after I merged it. I was going to see if this gets fixed with the next run when the next PR is merged.

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

Successfully merging this pull request may close these issues.

2 participants