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

little schemer: add new problem specs #1285

Closed
wants to merge 1 commit into from
Closed

little schemer: add new problem specs #1285

wants to merge 1 commit into from

Conversation

FTLam11
Copy link

@FTLam11 FTLam11 commented Aug 4, 2018

Hi, this is a new problem request based around basic recursion. The main inspiration for this exercise comes from the book Little Schemer. Looking forward to feedback. Thanks!

"list": "'Zang'"
},
"comments": [
"The primitive null? is defined only for lists."
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't test invalid input error conditions

"property": "equality",
"input": {
"function": "car",
"list": ["Immaculate", 23, "G.O.A.T"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Either use all strings or numbers in the list, we're not testing lists.

@Insti
Copy link
Contributor

Insti commented Aug 4, 2018

You also need the problem description and metadata files. You did :)

"description": "null of non-empty list",
"property": "boolean",
"input": {
"function": "null?",
Copy link
Contributor

Choose a reason for hiding this comment

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

Those from languages where ? is not a valid identifier character will probably object to this.

"cases": [
{
"description": "null of non-empty list",
"property": "boolean",
Copy link
Contributor

Choose a reason for hiding this comment

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

property tends to describe the name of the function/method being tested.
In this case it would should probably be "is_null" and the function section of input should be removed.

"comments": [
"The primitive cdr is defined only for non-empty lists. The cdr of any non-empty list is always another list"
],
"expected": -1
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an established format for specifying errors, check the docs.

@@ -0,0 +1,16 @@
Implement four operations: `null?`, `car`, `cdr` (pronounced could-der), and
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit confusing since there are 5 things you need to implement.

@Insti
Copy link
Contributor

Insti commented Aug 4, 2018

While I appreciate the effort that has gone into this, I don't think this is a good exercise to add, it's a bit too specific algorithm.

See also: #761
And: exercism/discussions#124

(Unless these policies have changed with V2?)

@petertseng
Copy link
Member

I infer that the ultimate goal of this exercise is to write the function to determine whether a given list is a list of atoms. I infer this using a few things:

A comment in the JSON file:

sequential build up of primitives for use in writing the final recursive function lat?.

In the description:

to be used together in defining a recursive operation lat? (short for list of atoms).

Given this is the case, I would suggest at least two things:

Exercise name

I'd suggest the little-schemer change to something indicative of the exercise content, such as list-of-atoms.

The reason is that the name little-schemer will be automatically converted to "Little Schemer" and displayed on https://exercism.io/my/tracks/track-name-here , and it would be useful to be able to know at a glance what the exercise is about from there.

This will make more sense to those who have not read that particular book. Even for those who have read that particular book, if the book has more than this particular exercise then the more specific name also serves to disambiguate.

Intermediate functions

Testing the intermediate functions (null?, car, cdr, atom?) goes against the current idea that exercises should not be prescriptive (exercism/discussions#44). If following that idea, the description would only mention the lat? function and none of the others, and the tests would also only test lat?. After all, who's to say lat? can only be implemented that way? There could be other ways and it would be valuable to see them all.

"property": "boolean",
"input": {
"function": "atom?",
"list": ["Immaculate", 23]
Copy link
Member

Choose a reason for hiding this comment

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

since it is expected that inputs to atom? may be things other than lists, I would say to use something reflective of that fact. Perhaps input, or if that's deemed too redundant (having an input nested inside input) then value.

@FTLam11
Copy link
Author

FTLam11 commented Aug 8, 2018

@Insti @petertseng Thank you both for all the helpful feedback. The linked discussions were insightful. I'll rethink this exercise through and see if it's still worthwhile.

@ErikSchierboom
Copy link
Member

While I appreciate the effort that has gone into this, I don't think this is a good exercise to add, it's a bit too specific algorithm.

I agree with @Insti here. The exercise also feels like it has some overlap with the existing list-ops exercise.

@Insti
Copy link
Contributor

Insti commented Aug 25, 2018

I'll close this since it's been a while with no activity.
Thanks for your work on this @FTLam11
Feel free to re-open or make a new PR if you find a way to make this exercise more Exercism-like.

@Insti Insti closed this Aug 25, 2018
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.

4 participants