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

alternateSign function #57

Closed
romcar opened this issue Apr 12, 2018 · 9 comments
Closed

alternateSign function #57

romcar opened this issue Apr 12, 2018 · 9 comments

Comments

@romcar
Copy link

romcar commented Apr 12, 2018

Hello,
I'm working on the recursion prompts and noticed that the test being done on alternateSign are not working according to specifications.

// their original sign. The first number in the index always needs to be positive.
// alternateSign([2,7,8,3,1,4]) // [2,-7,8,-3,1,-4]
// alternateSign([-2,-7,8,3,-1,4]) // [2,-7,8,-3,1,-4]

After completing the function I noticed that the first element being positive gave off an error. I adjusted the functions code to make the first element in the array negative and then alternate from there. I guess I just wanted to let someone who made it know.

Regards,
Erwin

@romcar
Copy link
Author

romcar commented Apr 12, 2018

Nevermind, I see what I did. It was a simple ternary statement. Although it is very strange that I could still pass those test with incorrect values. Sorry about that!

@romcar romcar closed this as completed Apr 12, 2018
@mybrainishuge
Copy link
Contributor

mybrainishuge commented Apr 12, 2018

Thanks for messaging me, @romcar. I'm glad you figured it out on your own. Do you mind providing more detail for the false positive you received? It looks like there may have been a bug in Chai's deep equality assertion. I'm wondering if you came across a case where their algorithm for checking deep equality failed.

@mybrainishuge mybrainishuge reopened this Apr 12, 2018
@romcar
Copy link
Author

romcar commented Apr 12, 2018

Well here's my code:

var alternateSign = function(array) {
  let temp = array.slice();
  let element = temp.shift();

  if(array.length === 0){
    return [];
  }

  //if array length is odd make the element positive 
  if(array.length%2 !== 0){
    //is the number positive 
    return [element > 0 ? 0-element : element].concat(alternateSign(temp));
  } else {
    //if the array length is even make the element positive

    //is the element negative
    return [element < 0 ? 0-element: element].concat(alternateSign(temp));
  }
  
  return temp;
  /*Almost gave up on this one*/
};

I checked the chai code and it tests to see if the array starts with a positive number. With my code, as the problem recursively flows it determines the sign of the element in the array according to the length of the array. Right now my arrays come out starting with negative values then continuing alternation from there.

The second if statement (array.length) can be switched to === to make the array being with a positive number. But the test will fail. I'm sure it's a bug involving the deep equality assertion. No matter how I changed my code it will only pass if the alternation is neg, pos, neg, ...

@mybrainishuge
Copy link
Contributor

Is this your corrected code or the code that returned the false positive? @romcar

@romcar
Copy link
Author

romcar commented Apr 13, 2018 via email

@mybrainishuge
Copy link
Contributor

I ran the code you provided above and it returns the correct output. Can you help me produce the false positive you saw? @romcar

@romcar
Copy link
Author

romcar commented Apr 13, 2018

When I type in:

alternateSign([3, 3, 4])
-->(3) [-3, 3, -4]```

I ran a few console.log test and see that they come out positive but when I type in the above in the console I get a return value of [-3, 3, -4].

@mybrainishuge
Copy link
Contributor

For an input of [3, 3, 4], the output should be [3, -3, 4]. The numbers returned all even indices should be positive. The numbers returned in all odd indices should be negative. If that's not the output your implementation is returning, then I think you have a bug to fix. :)

This is not an input I included in the tests, so it can't be a bug in the tests. The tests I included are intended to be a guide, but don't account for all possible edge cases. If you want to add tests to account for some edge cases, I'd be thrilled for you to make a PR and I'll review it.

@romcar
Copy link
Author

romcar commented Apr 13, 2018

I accept the challenge, when I have more time as of right now I'm super busy. Sorry to take up your time. Thanks for responding though.

@romcar romcar closed this as completed Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants