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

Prototype Literal::Array #133

Merged
merged 4 commits into from
Nov 9, 2024
Merged

Prototype Literal::Array #133

merged 4 commits into from
Nov 9, 2024

Conversation

joeldrapper
Copy link
Owner

@joeldrapper joeldrapper commented Nov 8, 2024

Initial stab at ##134.

@joeldrapper joeldrapper force-pushed the prototype-literal-array branch from 600bf25 to 3764c17 Compare November 8, 2024 23:05
Comment on lines 20 to 27
def initialize(type, value)
unless Array === value && value.all?(type)
raise
end

@__type__ = type
@__value__ = value
end
Copy link
Owner Author

Choose a reason for hiding this comment

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

We should probably allocate and call another method because this shouldn’t be called directly by a user.

Comment on lines +8 to +10
def new(*value)
Literal::Array.new(@type, value)
end
Copy link
Owner Author

Choose a reason for hiding this comment

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

This importantly makes a copy of the array since it picks up the splat. This is essential to maintain the integrity of the array, since otherwise it could be mutated elsewhere. This value should never be exposed. Even to_a should only expose a duplicate.

@joeldrapper joeldrapper marked this pull request as ready for review November 9, 2024 10:40
@joeldrapper
Copy link
Owner Author

I’m going to merge this now so work on other methods can be parallelised.

@joeldrapper joeldrapper merged commit 1287b36 into main Nov 9, 2024
10 checks passed
@joeldrapper joeldrapper deleted the prototype-literal-array branch November 9, 2024 10:41
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.

1 participant