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

key-comparison maze of arrays unclear test of identity or internal structure #798

Closed
driftsignal opened this issue Mar 29, 2024 · 3 comments · Fixed by #803
Closed

key-comparison maze of arrays unclear test of identity or internal structure #798

driftsignal opened this issue Mar 29, 2024 · 3 comments · Fixed by #803

Comments

@driftsignal
Copy link

the equality test for maze of arrays appears to test for identity.

key-comparison-test.lisp line 115-118:

    (:room-arrays
      . (((,+an-array+ ,+a-similar-but-different-array+) . explosion)
      ((,+an-array+ ,+an-array+) . victory)))

the readme suggests internal comparison:

6. The maze of arrays

This maze is simpler with only two rooms. The first needs a key that checks if the arrays contain the equal contents. The second needs a key that is more flexible about checking equality of numbers.

For example:

a                        ; => #[13 23]
b                        ; => #[13 23.0]

(key-arrays a b)         ; => NIL
(key-arrays-loosely a b) ; => T

because a permissive equality predicate will not consider numeric type when comparing the contents of an array.

would it make sense for the readme or the tests be updated to mirror each other?

Copy link

Hello. Thanks for opening an issue on Exercism 🙂

At Exercism we use our Community Forum, not GitHub issues, as the primary place for discussion. That allows maintainers and contributors from across Exercism's ecosystem to discuss your problems/ideas/suggestions without them having to subscribe to hundreds of repositories.

This issue will be automatically closed. Please use this link to copy your GitHub Issue into a new topic on the forum, where we look forward to chatting with you!

If you're interested in learning more about this auto-responder, please read this blog post.

@verdammelt verdammelt reopened this Mar 31, 2024
@verdammelt
Copy link
Member

Thanks for reporting this - I will check it out.

@verdammelt
Copy link
Member

@driftsignal I updated the test data and instructions to hopefully remove the possible confusion. Please review #803 and let me know if you think it corrects this problem.

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 a pull request may close this issue.

2 participants