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

Question: Should we change our stubs policy? #112

Open
joshgoebel opened this issue Jul 27, 2022 · 8 comments
Open

Question: Should we change our stubs policy? #112

joshgoebel opened this issue Jul 27, 2022 · 8 comments

Comments

@joshgoebel
Copy link
Contributor

Pulling this out incase it requires further discussion. Didn't want it to get lost.


The first example (just new) only works [well] in this case because ALL tests start with a call to the new method... if there were other static methods and those did not exist... then they would need to exist or you get a hard error.

The second example - the hard error (Wren Console can't compile the script or test suite)... I don't think is super welcoming for students. Would/should it be reasonable for harder exercises? I dunno... I was trying VERY hard not to have any "boiler plate comments" which students never remove and drive me nuts.

Or maybe my first assertion above is wrong - if we went with something like this:

class HighScores {
}

You get a soft runtime error of:

HighScores metaclass does not implement 'new(_)'.
at test(_,_) block argument (./high-scores.spec.wren line 7)

I think static methods may be resolved at compile-time though?

Originally posted by @joshgoebel in #111 (comment)

@glennj
Copy link
Contributor

glennj commented Jul 27, 2022

In List-Ops, if I comment out the static concat method, and then do.test the first concat test, I get the runtime error

$ wrenc list-ops.spec.wren
...
● concatenate a list of lists -> empty list

ListOps metaclass does not implement 'concat(_)'.

  87 |   do.describe("concatenate a list of lists") {
  88 |     do.test("empty list") {
> 89 |       var list1 = ListOps.concat([])
  90 |       Expect.value(list1.toList).toEqual([])
  91 |     }

at test(_,_) block argument (./list-ops.spec.wren line 89)

Tests:  😤 ✕ 2 failed, ☐ 26 skipped, ✓ 0 passed, 28 total

Failing tests.
at (script) (./list-ops.spec.wren line 206)
at test(_,_) (wren-testie/testie line 31)
at run() (wren-testie/testie line 93)

In the case where I comment out the whole class, then it's the import that fails

$ wrenc list-ops.spec.wren
Could not find a variable named 'ListOps' in module './list-ops'.
at (script) (./list-ops.spec.wren line 2)

Similarly, if I rename the solution file, import fails

$ wrenc list-ops.spec.wren
Could not find a variable named 'ListOps' in module './list-ops'.
at (script) (./list-ops.spec.wren line 2)

I don't know if I would characterize that as "compile-time": to me it just fails before Testie starts testing.

It's a shame that we get "Could not find a variable named 'ListOps' in module './list-ops'." for both missing file and missing class.

@joshgoebel
Copy link
Contributor Author

joshgoebel commented Jul 27, 2022

I don't know if I would characterize that as "compile-time": to me it just fails before Testie starts testing.

Ah, you may be right (since import errors can be captured at runtime)... but I think it depends on the nuance of what we mean by compile time... if we're just talking about whether ANY code has run yet, they yes... top-level code that's defined in wren-testie should have been run (since testie is imported first). But when import list-ops is hit (with no rescue), then the Wren VM crashes and you get the harder-error... If the file was there it would first be parsed/compiled, then executed... so much like Ruby it's all happening a bit dynamically.

It's a shame that we get "Could not find a variable named 'ListOps' in module './list-ops'." for both missing file and missing class.

Please file that as a bug/issue against wren-console... that sounds buggy maybe - or at least something we could explore fixing since imports are handled by the internal Wren runtime, so we have a ton of flexibility for how we'd deal with them...

For Exercism though is that a common issue - or you're debating perhaps in some cases NOT even giving someone a stub file at all?

@glennj
Copy link
Contributor

glennj commented Jul 28, 2022

I think we need something in between. I like the way Java does it: once an exercise gets to a certain difficulty, make the student implement everything -- no stubs.
https://github.com/exercism/java/blob/main/exercises/practice/alphametics/src/main/java/Alphametics.java

That would mean we'd need to reconsider the difficulties and ordering of wren exercises.

re: boilerplate comments: I added a check in the bash analyzer that will warn if they're still there.

@joshgoebel
Copy link
Contributor Author

joshgoebel commented Jul 28, 2022

Not opposed but I really really really hate stub comments - students never remove them in my experience - that's why none of the Wren files have a stub comment (except for the very first exercise)... I wonder if we made it part code if that would help:

class DeleteMeWriteYourOwn {
/*
Since this exercise has a difficulty of > 4 it doesn't come
with any starter implementation.
This is so that you get to practice creating classes and methods
which is an important part of programming in Java.
Please remove this dummy class and then start writing your solution
*/
}

@joshgoebel
Copy link
Contributor Author

Or are you really opposed to just include the class X {} and nothing else? (which would remove the need for a stub)

@glennj
Copy link
Contributor

glennj commented Jul 28, 2022

No, that would be great. I'd say we could do that for any exercise with difficulty > 2.

@joshgoebel
Copy link
Contributor Author

My thoughts then:

  • make sure sequentially the first ~ 4-6 are difficulty < [threshold] (to let someone get up to speed)
  • should we check with other tracks to see if there is a common value of threshold, or have you already done this?

Does that sound like a plan?

@glennj
Copy link
Contributor

glennj commented Jul 28, 2022

Yes, easiest first.

I kind of have, in the Tcl and bash tracks.

Probably not too difficult to take the average difficulties from some other tracks: a bit of find | jq chicanery.

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

No branches or pull requests

2 participants