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

feat: Document arrays and loops #341

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: Document arrays and loops #341

wants to merge 2 commits into from

Conversation

ospencer
Copy link
Member

Closes #339

@ospencer ospencer requested review from a team December 17, 2022 23:22
@ospencer ospencer self-assigned this Dec 17, 2022
@netlify
Copy link

netlify bot commented Dec 17, 2022

Deploy preview ready! 🌾

Name Link
🔨 Latest commit a75ef84
🔍 Latest deploy log https://app.netlify.com/sites/grain-lang/deploys/639e58f82c9bc90008c17514
😎 Deploy Preview https://deploy-preview-341--grain-lang.netlify.app/docs/guide/arrays
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

src/guide/arrays.md Outdated Show resolved Hide resolved
src/guide/arrays.md Outdated Show resolved Hide resolved
- efficient at adding additional elements
- efficient at removing elements
- easy to work with
- less succeptible to bugs
Copy link
Member

Choose a reason for hiding this comment

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

In what way?

Copy link
Member Author

Choose a reason for hiding this comment

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

No possible IndexOutOfBounds, typically pattern matching on them which forces you to handle empty cases, etc. I was fine letting the guide have this as "take our word for it" but we could expand on that if we wanted. Wouldn't do it inline though.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, expanding might be valuable because "take our word for it" seems like a bad position for our "guide" 😛

src/guide/arrays.md Outdated Show resolved Hide resolved

Lists and arrays are similar constructs, but each has its own pros and cons. It's up to you to choose which is right for your use case!

Lists are excellent because they're
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Lists are excellent because they're
Lists are excellent at

Copy link
Member

Choose a reason for hiding this comment

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

I thinkthere was a better transition here because, it goes on to list the properties rather than use cases. at indicates listing specific examples. It might make sense to say something like

Lists are useful over arrays because they are immutable, 
and their data structure makes resizing them more efficient,
they are easier to work with in grain and less susceptible to bugs 
because it is a lot harder to get runtime errors. The downfalls of lists compared 
to arrays are that they are unable to modified in place, and their data structure 
makes accessing specific elements and determining the length less efficient. 
Lists are best used when the size does not matter and you are constantly 
appending or removing elements.

The downfall of that sentence structure is you lose the nice list though a better option over a list might be having the two paragraphs with their differences and then using a table to compare the features.

- inefficient at accessing random elements
- inefficient at determining the number of elements

Arrays are excellent because they're
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Arrays are excellent because they're
Arrays are excellent at

Copy link
Member

Choose a reason for hiding this comment

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

I guess the list would have to be reworded if these are both changed to "at" but I think it's best to say what they are "excellent at"


- inefficient at adding additonal elements
- inefficient at removing elements
- more succeptible to bugs
Copy link
Member

Choose a reason for hiding this comment

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

How?

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
@marcusroberts
Copy link
Member

All read very well to me!

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Still some of my inline comments to address. Sorry for not batching as a review.

- inefficient at removing elements
- more succeptible to bugs

As a rule of thumb, lists are the idiomatic choice in Grain! Lean towards using lists whenever possible and use arrays when it makes your code easier to understand.
Copy link
Member

Choose a reason for hiding this comment

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

You might want to state that Lists are more idiomatic because they are generally easier to work with and more feature rich supporting destructuring in pattern matches and the fact that they are less likely to throw runtime errors. you might also want to note that as grain is functional lists are the better choice because you they are immutable.

Choose a reason for hiding this comment

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

Suggested change
As a rule of thumb, lists are the idiomatic choice in Grain! Lean towards using lists whenever possible and use arrays when it makes your code easier to understand.
As a rule of thumb, if you have to keep track of the amount of elements or want to index to retrieve an element often, arrays are the best choice. However, you should use lists in (tail) recursive applications! Lean towards using idiomatic lists whenever possible and use arrays if you need length or random access.

This rule of thumb more accurately paints the picture of determining the most efficient data structure for the use case.

@spotandjake
Copy link
Member

This also probably closes #244


- inefficient at adding additonal elements
- inefficient at removing elements
- more succeptible to bugs

Choose a reason for hiding this comment

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

Suggested change
- more succeptible to bugs
- more susceptible to bugs

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.

document loops
5 participants