I'm applying TDD while creating my project, and I'm implementing a lot of tests and checks for my functions to ensure their arguments are correctly input.
I have no issue with this, but my code is starting to look congested with all the argument checks. Is this proper practice or is argument checking not generally implemented like this?
export default class Ship {
constructor(shipLength) {
if (typeof shipLength !== 'number') throw new Error('Length must be a number');
if (arguments.length !== 1) throw new Error('Must enter only 1 argument');
if (shipLength < 1) throw new Error('Length must be greater than 0');
if (shipLength % 1) throw new Error('Length must be an integer');
this.length = shipLength;
this.hits = Array(this.length).fill(false);
}
hit(location) {
if (typeof location !== 'number') throw new Error('Must be a number');
if (arguments.length !== 1) throw new Error('Can only accept one argument')
if (location > this.length - 1) throw new Error('Must be within ship body');
if (location < 0) throw new Error('Cant be negative');
if (location % 1) throw new Error('Must be an integer');
this.hits[location] = true;
}
}
import Ship from "../src/ship";
describe('Test that the constructor', () => {
it('accepts only one parameter of type number', () => {
expect(() => new Ship(3)).not.toThrow();
expect(() => new Ship(1,2,3)).toThrow();
expect(() => new Ship([1,2,3])).toThrow();
expect(() => new Ship('asd')).toThrow();
});
it('doesnt take < 1 as length', () => {
expect(() => new Ship(0)).toThrow();
expect(() => new Ship(-1)).toThrow();
});
it('only accepts integers', () => {
expect(() => new Ship(1.1)).toThrow();
});
it('sets the passed ship length', () => {
expect(new Ship(3).length).toBe(3);
});
it('creates a hit array', () => {
expect(new Ship(3).hits).toEqual([false, false, false]);
})
});
You should do runtime validation of user supplied values. Because those only appear at runtime and errors in them must be handled at runtime. Doing this for values which are passed internally in your code is usually overkill. Because while those errors will surface at runtime, you cannot do anything about them at runtime; you need to fix them by fixing your code. So runtime checks for those values might help you discover bugs in your code, but at the cost of very convoluted code; and it's not even guaranteed to easily help you discover those bugs, if your code doesn't happen to take that particular path.
What does help unearth these types of bugs at development time is a static type checker like TypeScript. Using it often makes more sense than these runtime type checks.
Don't look at whether the code is overwhelmed is a good practice if it leads to safe code running without errors only
edit: sure, and you could use some kind of library, but this way you have access to what you check and how you check it, which makes it easier to modify later if necessary.