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

Make basic_filter copy a little clearer #147

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

clamey
Copy link

@clamey clamey commented Feb 15, 2016

No description provided.

Copy link
Owner

@timoxley timoxley left a comment

Choose a reason for hiding this comment

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

Confusion perhaps comes from the Example not being clear about that being output, not input.

Perhaps it simply needs explicit "example input" and "example output".

{ message: 'Mollit ea labore do laboris nostrud proident ullamco.' },
{ message: 'Laboris excepteur aliquip ullamco culpa.' },
{ message: 'Culpa minim proident dolor ipsum ullamco est qui non aute aliquip excepteur sunt.' },
{ message: 'Cupidatat duis sit aute pariatur amet sit excepteur cupidatat.' } ]
Copy link
Owner

Choose a reason for hiding this comment

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

This is incorrect output though, the solution expects an array of strings, not an array of objects.

@@ -1,7 +1,7 @@
# Task
Use Array#filter to write a function called `getShortMessages`.

`getShortMessages` takes an array of objects with '.message' properties and returns an array of messages that are *less than < 50 characters long*.
`getShortMessages` takes an array of objects with 'message' properties and returns an array of messages that are *less than < 50 characters long*.
Copy link
Owner

Choose a reason for hiding this comment

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

If this said "an array of messages as strings" I'd agree that this is clearer.

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