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

word-search: Implement canonical-data.json #591

Closed
kytrinyx opened this issue Feb 13, 2017 · 9 comments
Closed

word-search: Implement canonical-data.json #591

kytrinyx opened this issue Feb 13, 2017 · 9 comments

Comments

@kytrinyx
Copy link
Member

We want to have a standard set of test inputs and outputs for each exercise to
make it easier to port them to new languages, as well as to help keep the
exercises in sync.

The Word Search exercise can be found in the ./exercises/word-search/ directory.

The step-by-step instructions for how to compile a canonical-data.json file is
described in this section of the contributing guide.

See http://exercism.io/contribute/canonical-data/word-search for the up-to-date list of
language tracks that have an implementation of the Word Search exercise.

The tracking issue has more context.

@stkent
Copy link
Contributor

stkent commented Feb 26, 2017

Existing implementations:

xcsharp added: 2016/09/13
xfsharp added: 2016/05/10
xgo added: some time before 2016/01/22
xlua added: 2016/03/20
xpython added: 2016/10/31

The exercise description includes the following grid:

gefblpepre
cbmdcimguc
oikoknrjsm
pbwjrqrota
rixilelhgs
woncqlispc
schemekmgr
alxhprubyi
javaocamlp
clojurermt

xgo uses the description grid.

xcsharp, xfsharp, xlua, xpython all use the following grid:

jefblpepre
camdcimgtc
oivokprjsm
pbwasqroua
rixilelhrs
wolcqlirpc
screeaumgr
alxhpburyi
jalaycalmp
clojurermt

which differs subtly from the example grid:

screen shot 2017-02-25 at 9 18 46 pm

The git history for the exercise description does not reveal how this deviation came about. However, the xgo implementation is the earliest of the 5 tracks, so I posit that a track decided to tweak the description grid (for reasons unknown to me) and that other tracks duplicated the changed grid rather than the description grid.

cc @ErikSchierboom: you added the C# and F# implementations of this exercise; do you recall how or why the description grid was not used in those cases? Thanks!

@petertseng
Copy link
Member

Is it the case that the C# and F# implementations deliberately test all eight directions?

If so, should that become the standard?

Does the existing example grid test all eight directions?

@stkent
Copy link
Contributor

stkent commented Feb 26, 2017

@petertseng I was just diving in to that, and yes: C# and F# test all directions, and C# (where I started reading) also checks a grid of a different size:

qwertyuiopz
luamsicrexe
abcdefghijk

@stkent
Copy link
Contributor

stkent commented Feb 26, 2017

C# also tests that words not in the puzzle are not found 👍

@stkent
Copy link
Contributor

stkent commented Feb 26, 2017

Ok, @petertseng, you nailed it: Go checks 4 of the 8 available directions.

E
S
W
NW

I'll use the updated version of the grid for the canonical tests.

@petertseng
Copy link
Member

Perhaps the example grid can also get changed, then.

The other interesting corner case I would recommend is... literally the corner case. Check that a word is found even if one of its letters is in the bottom-right corner. Just checking those off-by-one errors.

@ErikSchierboom
Copy link
Member

ErikSchierboom commented Feb 26, 2017

@stkent I'm not completely sure, but I think I copied the existing tests from the Python test suite, where I must have failed to regard the example data in the description. I do think that testing for all directions makes sense here, as that is also how one usually solves these puzzles. Having some corner cases is always good, so I'm all for that!

Perhaps the example grid can also get changed, then.

+1

@stkent
Copy link
Contributor

stkent commented Mar 1, 2017

Here's a visualization of the current (newer) solutions. All corners are accounted for!

screen shot 2017-02-28 at 9 27 27 pm

@stkent
Copy link
Contributor

stkent commented Mar 1, 2017

I opened a PR to update the description example: #617. I think the description could benefit from other refinements, but figured this was an easy change to make that might save someone from implementing the 'old' version while the canonical data is under construction :)

emcoding pushed a commit that referenced this issue Nov 19, 2018
generators: Exercise cases helper methods

    moved ExerciseCase under the Generator module (and gave thanks for sed)
    moved the assert helper methods out into their own module, for ease of testing and because ExerciseCase was becoming bloated.
    added test coverage for ExerciseCase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants