7 kyu

Squeaky Clean

1,474 of 1,475user1496713
Description
Loading description...
Arrays
Fundamentals
View
AllIssues14QuestionsSuggestions4Show Resolved
  • Please sign in or sign up to leave a comment.
  • ejini战神 Avatar

    We don't need another lame, trival, low quality kata that does the same thing... --> https://www.codewars.com/kata/search/my-languages?q=filter&order_by=sort_date%20desc

    I might as well post another wall of filtering based katas that I have collected (not done yet) here later to give some form of insights @glovastefan

  • JohanWiltink Avatar

    Needs update to current Node version.

  • le-cr Avatar

    Change the "var" in the description to "let".

  • JohanWiltink Avatar

    Intent was apparently to filter falsy values. It has apparently missed false, -0 and NaN since creation.

    There is now also 0n, so the kata is now ignoring fully half of existing falsy values. ( So are some, though not all, solutions. )

    It is unspecified what to do about sparse arrays, which might be considered a special case of arrays with falsy values ( but that's probably solidly out of scope anyway ).

  • FArekkusu Avatar

    Trivial map/filter/reduce is not a novel idea, and I think there's already at least one kata asking to imlement an exactly same thing.

  • SandQueen Avatar

    Before someone approves it: random tests are missing.

    In case the current test cases are seen as enough, feel free to resolve this issue :)

  • rawman9x Avatar

    cool!

  • Vidmas Avatar

    Please correct word grammar, like "exisit" and "srting".

  • brburzon Avatar

    There was a typo on the instructions.

  • antoxann Avatar

    nice cata, helps me to learn new javascript method

  • mrageh Avatar

    The descriptions for two of the tests are incorrect. The point of the kata is to filter out falsey values from an array, yet one of the tests says should ignore non-zero numbers. But it should it only ignore numbers that are zero.

  • ankitk07 Avatar

    In the test case window, if you are typing in the last line, horizontal scroll bar appears over text your are typing.

  • ankitk07 Avatar

    Initial test case execution timedout because servers were busy.

  • deanvn Avatar

    suggested initial tests cases

    testArray('Testing Valid Values', 'should ignore non-zero numbers', squeakyClean([1,2,3,0,1.1]), [1,2,3,1.1]); testArray('Testing Valid Values', 'should ignore non-empty strings', squeakyClean(['hello', '14', '']), ['hello', '14']);

  • zetsubo Avatar

    Typo in problem statement.

  • DeanBrown Avatar

    This comment has been hidden.

  • computerguy103 Avatar

    This is totally not necessary:

    It should test for valid values, invalid values and edge cases.

    Example scenario:

    You are building a web app that records user interactions for analytics. Your app should only be interested in real values. For example, 'cta-clicked' or 'header-clicked' would be acceptable values that you'd want your app to store in an array.

    The rest of the description provides plenty of information to solve this Kata. That part I quoted just makes it more confusing and I think it should be removed.

  • everdimension Avatar

    Kata description is a little misleading: it gives an impression that the clean function should return an array of removed values, while it is expected to return the filtered array of "truthy" values.

  • user6235512 Avatar

    Should false be cleaned or not? If not, you need a test case.

  • greghmerrill Avatar

    This statement feels vauge: "and sometimes undesireable values like empty strings, 0, null, or undefined are stored during the session"

    When you say "values like" it implies that this may not be the full set of cleanups required. It would be better if you were to clearly state that "squeakyClean should return a new array with all empty strings, 0, null and undefined removed".

  • wthit56 Avatar

    Was unsure if you wanted the original array cleanup, in-place, or a new array created with only "valid" values. Good idea, though.

  • jacobb Avatar

    OverZealous has already mentioned just about everything I would.

  • OverZealous Avatar

    Sorry about the wall of text here. I got carried away. Most of this is somewhat general suggestion, and you don't have to do it all on this kata. I do recommend at least the first section. :-)

    Your description refers to null or undefined values. However, your test code has the function filtering the empty string (''), which is not null or undefined. It's better to explicitly define all the cases which should be considered, rather than rely on the user to infer your interpretation. This also helps in writing better tests, as you have a list of explicity values to test against.

    You've also do not have edge cases for values, such as:

    • false
    • 0
    • '0'
    • []

    Which all will evaluate to false when checked via !!value. If you decide to add edge these cases, them you should also explicitly list these as well, and provide specific tests.

    When writing tests, the message should be useful, preferably describing what value is being tests. Otherwise it provides no value, and can be omitted.

    For example:

    Test.assertSimilar(cleanArray([1,2,'',null,5]), [1,2,5], "Failed")

    This provides no feedback to the user. It's a simple case, so you could either replace "Failed" with something better, such as "Should remove the empty string and null", or remove it completely, since assertSimilar will report the actual and expected values.

    Even better, you can use describe() and it() to group tests with descriptions, so you provide a sort of story for the user:

    describe('Testing Valid Values', function() {
  it('should ignore non-zero numbers', function() {
    Test.assertSimilar(cleanArray([1,2,3,-1,1.1]), [1,2,3,-1,1.1]);
  });
  it('should ignore non-empty strings', function() {
    Test.assertSimilar(cleanArray(['hello', '14']), ['hello', '14']);
  });
});

describe('Testing Invalid Values', function() {
  it('should remove undefined', function() {
    Test.assertSimilar(cleanArray([1,,2,3,0,-1,1.1]), [1,2,3,-1,1.1]);
  });
  it('should remove null', function() {
    Test.assertSimilar(cleanArray([1,null,2,3,0,-1,1.1]), [1,2,3,-1,1.1]);
  });
  it('should remove 0', function() {
    Test.assertSimilar(cleanArray([1,2,3,0,-1,1.1]), [1,2,3,-1,1.1]);
  });
  it('should remove empty strings', function() {
    Test.assertSimilar(cleanArray(['hello', '14', '']), ['hello', '14']);
  });
});

describe('Testing Edge Cases', function() {
  it('should handle all invalid values', function() {
    Test.assertSimilar(cleanArray(['',0,null,undefined]), []);
  });
});

describe('Randomized Test Cases', function() {
  // ... etc ...
});

    Doing this not only makes your tests clearer and provide better coverage, but building up the tests from simpler to more complex helps the user find bugs faster and easier, rather than tracking down typos and other errors mixed in with more complex tests. Specifically, notice how I've tested against specific values, rather than mixing in multiple values to filter. This way, if the code works on undefined but not null, I have a specific test to compare against.

    Tip: to make this cleaner, write a helper function that takes a message, input, and output, and handles calling it() for you, like so:

    function testArray(msg, input, output) {
  it(msg, function(){ Test.assertSimilar(cleanArray(input), output) });
}

// usage
testArray('should ignore non-zero numbers', [1,2,3,-1,1.1], [1,2,3,-1,1.1]);

    This might seem like a simple kata, but, if I were writing this method from scratch, I can think of a few of ways to fail that might pass simple tests, but miss edge cases:

    • All invalid
    • Only invalid at the beginning or at the end (off-by-one errors)
    • Modifying the original array (Is this good or bad? It should be explicitly defined in your spec, and tested.)

    Also, I recommend you wrap your example code in single backticks (e.g., `null`) for inline code, and you can use three or more backticks (or indentation) to format code blocks:

    ```js
var originalArray = ['click1','click2',null,'','','submitForm'];
```

    or indentation:

        var originalArray = ['click1','click2',null,'','','submitForm'];

    I personally prefer multiple backticks, even for single lines, as you can specify the language (note the js after the initial set of backticks).