# Tag Info

5

First, your function should follow JS conventions and be camelCase, not PascalCase. The latter is for constructors, but this is just a function. So for the following I'll call it setAttributes instead. As for your tests/spec: You should structure - and name - them differently. Treat the descriptions as documentation you can read through. E.g. the describe ...

5

There is a practical limit to take into account when it comes to using recursive functions: the size of the call stack. If the depth of the nested arrays is not limited, you can not use a recursive solution in javascript because most of the time you cannot assume that the interpreter has tail call optimizations: https://stackoverflow.com/questions/37224520/...

5

The check for arr.length > 0 is not needed: forEach() can be called on an empty array (and does nothing in that case). The check if (arr) protects against a call with an undefined argument, but not against a call with a non-array. As an example, flatten(123) would cause a JavaScript error. So you have to decide: If the argument to flatten() is known to ...

3

From a once over: I would drop this test, you are testing Jasmine and/or the test, but not the results of calling app.navigation.init() it ('spy on behavior', function () { var spy = spyOn(app.navigation, 'init'); app.navigation.init(); expect(spy).toHaveBeenCalled(); }); Be aware that Function.bind() which you use in navigation.init() creates ...

3

Here's a recursive function whose basic idea is similar to yours: function flatten(a) { if (! Array.isArray(a)) throw new Error('flatten() called with non-array'); const f = []; function p(a) { for (let e of a) if (Array.isArray(e)) p(e); else f.push(e); } p(a); return f; } Advantages of this version: It avoids copying the nested arrays ...

3

Problems with recursion and cyclic infinity. BTW the best solution is array.flat(10); 10 is recursion depth. See bottom of answer for details on the new Array prototypes. Call stack overflow Your code is not protected from call stack overflow, As amard's answer correctly pointed out recursion is dangerous as the call stack depth is unknowable from ...

3

I found simpler solution for this specific case where the array items are integers. const flatten = array => JSON.stringify(array) .match(/\d+/g) .map(x => parseInt(x)) I wanted to know if this solution is faster or slower than the loop solution. So I wrote this script const generate = length => length <= 0 ? [] : ( Math....

3

You don't really need to use ng-html2js since you don't really have a file .html file. Answered on question 1. You can just use angular.mock.module if you want. I usually use this pattern for my tests. Example: const angular = window.angular, expect = window.chai.expect, sinon = window.sinon; const inject = angular.mock.inject, module = angular.mock....

2

On the principal that functions should do one thing and do it well, consider breaking out some pieces of your filter into smaller functions for readability. Filter (function () { 'use strict'; angular.module('example') .filter('formatTemperature', formatTemperatureFilter); var degreesSymbol = '\u00B0'; function ...

2

To begin with, don't interact with the DOM. The reason for that is imagine that you have hundreds of unit tests. Do you want to clean up the DOM after them? Are you going to forget sometimes? If you forget will you be able to find why unit tests are failing? When you will be setting up a Continuous Integration process which will run the unit tests will you ...

2

You can use array.map to create errs instead of creating an array and pushing to it. const errs = response.data.map(entry => options. \${entry.dataField}) With that out of the way, there's only two things that can happen with this code: No errors emitted, app goes to top, processing is false. Error emitted, app goes to top, processing is false. ...

2

From a once over and reading some Jasmine docs: I am not a big fan of _db and _testContext, I see no reason for the underscores. It is not guaranteed that _db will be set in new MyCtorFunction(_db).myMethod(_testContext.assertions);, so you should check for that prior to calling. You did not tell us what database (API) you are using, so we cannot tell ...

2

I agree with what you said, i.e.: The only thing which can break these tests is a change to this specific method and surely if anybody is changing this method they are doing so with good reason, and all I've done is given them an extra job of changing the tests. I can definitely say that writing these tests caused me to make several improvements ...

2

I cannot figure out better solution than yours, but I can suggest more concise code, though less readable. function flatten(arr) { if (!Array.isArray(arr)) { return []; } return arr.reduce(function(acc, x) { return acc.concat( Array.isArray(x) ? flatten(x) : [x] ); }, []); };

2

If you really want to unit test that logic, you should fake its data. In order to do that, you need a way to inject that dependency instead of getting it straight from it. You have 3 options: Installing a framework to mock the import (as detailed in https://medium.com/@emandm/import-mocking-with-typescript-3224804bb614) Have a constructor that is ...

1

Since router object is a mock, and the fact that it calls some method - without checking its result You have a unit test, more specifically a whitebox test, as opposed to a blackbox test that tests the output of some method. For it to become an integration test, you would have use a router instead of a mock.

1

If you are using any I/O (in this case, local storage), you must have fallen into an integration test. A unit test could be that when logout is executed, the call to clearing the local storage is made (using an spy on localStorage). An integration test could be that you have data in the local storage and after you have logged out, you don't.

1

The anonymous functions and hardcoded values can be abstracted. For example: function click(value){ utils.clickHotspotElementAtIndex(view, value, Function); } function tests(value){ expect(utils.verifyHotSpotHasBeenHiglightedAtIndex(view, value)).toBe('true'); } function test () { [0, 1, 2].map(tests); } Then the asynchronous code can be ...

1

You should be using the Chai assert.throws operator instead (http://www.chaijs.com/api/assert/#method_throwsO so your code might look like: it('should throw exception if config.env.json is malformed', async () => { // Arrange: beforeEach const fakeFsStub = sinon.stub(File, 'readFileAsyc'); fakeFsStub.withArgs('./src/configuration/config.json')....

1

Just factor it out into a closure function patchJasmineForAsyncError(method: string) { let oldFn = window[method]; function create_logger(fn: Function) { return (done) => { let p = fn(); if (p instanceof Promise) { p.then(done).catch(e => { console.error(e); ...

1

Would it be appropriate to first map() I don't think so. This would only serve to make the computer iterate over each value twice. I don't see how that would make it simpler. it would fail at the very first element that is less than or equal to 30 preventing us to see what other values of an array are. If I understand correctly, each test can only ...

1

Array::some can be used to reverse the test logic and return earlier.

1

I would say that, of course, the first version when you split your tests into granular logical blocks is much better and far more explicit. There is even a practice to have a single assertion per test which sometimes is too strict of a rule, but it helps to follow the Arrange Act Assert test organizational pattern.

1

Your naming of variables is good, they're mostly descriptive and easy to follow. You could use reverse conditional to reduce the cyclomatic code complexity, doing that your code would look something like: module.exports = { meta: { schema: [] }, create(context) { return { MemberExpression(node) { if (!node.property || !node....

1

I did come up with one alternative which is as follows: function triggerEvents(debugElement: DebugElement, eventName: string, object: any){ debugElement.triggerEventHandler(eventName, object); } function expectAndReset(spy: Function, object: any, spyObj: jasmine.Spy) { expect(spy).toHaveBeenCalledWith(object); ...

1

Although this is one way of avoiding repetition, each it clause doesn't really describe what it is doing, just what is expected. Taking your first one for example (describe('numberInRangeToRgb',...) I would rather see: describe('numberInRangeToRgb', () => { it('should return black when the value is at the beginning of the range', ... it('should ...

1

This looks okay to me, the only thing I will comment on is this: if (response.error) { if (options.error) { options.error({ error: response.error }); } if (options.complete) { options.complete(); } ...

Only top voted, non community-wiki answers of a minimum length are eligible