Skip to content

Conversation

@bpurinton
Copy link
Contributor

This is my first attempt at a merge into the course material, and my first time requesting code review. So bear with me.

Problem

I wanted to patch the string_multiplication.rb test to prevent it from passing with the code

p "Ho" + "Ho" + "Ho"

Solution

I made a counter, looped through the file, and searched for the presence of *, with the condition that it doesn't occur on a commented line. So as long as there is one * in their code somewhere, it passes.

Why I request review

I'm not sure if my solution is good and logical, or if it could be improved. Also, there are still other ways I could imagine cheating the test like:

variable = 5 * 6
p "Ho" + "Ho" + "Ho"

but I'm not sure we need to worry about every single edge case.

@bpurinton bpurinton marked this pull request as draft February 4, 2023 00:29
@bpurinton bpurinton marked this pull request as ready for review February 4, 2023 00:29
Copy link
Contributor

@jelaniwoods jelaniwoods left a comment

Choose a reason for hiding this comment

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

@bpurinton Thanks for the PR!

but I'm not sure we need to worry about every single edge case.

I agree, we don't need to address every edge case in these tests.

I had one minor suggestion about the error message, but you can take it or leave it.

LGTM, you can merge when ready.

@bpurinton bpurinton merged commit 4953a3e into appdev-projects:main Feb 10, 2023
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.

2 participants