#4382 Bug Fix #4384
#4382 Bug Fix #4384
Conversation
There was a problem hiding this comment.
@govardhanshah456 thanks for working the issue #4382. I really like that you decided to rename the p1 and p2 member variables.
|
Hey, i have made requested changes, please have a look into it. |
There was a problem hiding this comment.
As a quick bug fix it looks really good. I suggest to add some tests (it is good to have a test case causing the original bug). You can use the commit suggestion button.
I know that this is out of scope for this PR, but I think it would be reasonable to make the MedianOfRunningArray a generic type, so one could compute a median of integers (and the result would be an integer), doubles (and the result would be a double) etc. @govardhanshah456 would you like to work on such an issue?
Co-authored-by: Piotr Idzik <65706193+vil02@users.noreply.github.com>
Co-authored-by: Piotr Idzik <65706193+vil02@users.noreply.github.com>
…java Co-authored-by: Piotr Idzik <65706193+vil02@users.noreply.github.com>
…java Co-authored-by: Piotr Idzik <65706193+vil02@users.noreply.github.com>
|
Hey, as you said its out of scope for this PR, what we can do instead is we can make new enhancement issue of making Median Generic and then work on it, that would appear more appropriate, and yes median would always be double for instance, (1,2) median would be (1+2)/2 which is double although input is int array. |
Fixed The Bug for the Test Case mentioned in linked Issue as well as made code more readable.
Closes #4382.