Skip to content

My solution #1

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

My solution #1

wants to merge 3 commits into from

Conversation

jdesrosiers
Copy link
Contributor

For those who are interested, here's my solution to qualification task.

I chose to implement the Basic output format. I started by writing my test suite. For each keyword, I created at least one passing and failing test based on what result I expected from that keyword. I then went through and updated each of the applicator keywords to collect errors and add them to the Output object that was being returned. That was enough to get my tests passing, but I noticed that there was a lot of duplication in the code I just added to the keyword handlers. In working to eliminate the duplication, I was able to move all of the error collection and output generation into the validateSchema function. I was pretty happy with that because it means all of the output code is in one place and there's no duplication. It also allows the keyword handlers to be able to just return a simple boolean instead of an output unit, which I was also very happy about.

Because this solution required some refactoring, it may be hard follow what I did by looking at the PR as a whole. You may find it easier to just look at the last commit to see the actual solution. Other than refactoring to return a boolean instead of an Output object, the only changes to the keyword handlers were to pass the errors array through. All the actual logic is in the validateSchema function.

This is an implementation of the Basic output format, but this approach works for the Detailed output format as well. As an exercise to make sure you understand, try to figure out the small change it would take to change this to an implementation the Detailed output format.

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.

1 participant