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

Implement _check #2282

Open
wants to merge 1 commit into
base: paritosh142
Choose a base branch
from
Open

Conversation

paritosh142
Copy link

Description

Refactored the calculator operations (add, subtract, multiply, divide) to use the _check function for type checking. This removes redundancy and improves code maintainability.

Changes

  • Implemented _check function in src/calculator.js
  • Updated all calculator operations to call _check for type validation
  • Updated tests in src/calculator.test.js to reflect the changes

Testing

  • Ran npm test to ensure all tests pass

@danthareja danthareja closed this Oct 21, 2024
@danthareja danthareja reopened this Oct 21, 2024
Copy link
Collaborator

@dantharejabot dantharejabot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great attempt, @paritosh142!

I would like to request a few changes before merging your work. Please review my comments below and make the appropriate changes to your code.

After you update your code locally, follow the instructions to save your changes locally and push your changes to your fork.

When you push your changes to your fork, I'll come back for another review.


There are 36 style guide violations in your contribution. I've marked them with inline comments for your convenience.

Please revisit your code and follow the style guide best practices.

Hint: You might be able to fix some issues automatically by running npm run lint -- --fix


All the tests are passing. Nice job!

// Then, invoke this function inside each of the others
// HINT: you can invoke this function with exports._check()
exports._check = (x, y) => {
if (typeof x !== "number") throw new TypeError(`${x} is not a number`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

exports._check = (x, y) => {
if (typeof x !== "number") throw new TypeError(`${x} is not a number`);

if (typeof y !== "number") throw new TypeError(`${y} is not a number`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

@@ -1,20 +1,20 @@
/* eslint-disable no-unused-expressions */
const calculator = require('./calculator');
const calculator = require("./calculator");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)


describe.skip('_check', () => {
describe("_check", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

beforeEach(() => {
sinon.spy(calculator, '_check');
sinon.spy(calculator, "_check");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

describe('subtract', () => {
it('should throw a TypeError if arguments are not numbers', () => {
expect(() => calculator.subtract(40, '2')).to.throw(TypeError);
describe("subtract", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

it('should throw a TypeError if arguments are not numbers', () => {
expect(() => calculator.subtract(40, '2')).to.throw(TypeError);
describe("subtract", () => {
it("should throw a TypeError if arguments are not numbers", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

expect(() => calculator.subtract(40, '2')).to.throw(TypeError);
describe("subtract", () => {
it("should throw a TypeError if arguments are not numbers", () => {
expect(() => calculator.subtract(40, "2")).to.throw(TypeError);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

expect(() => calculator.subtract(40, [])).to.throw(TypeError);
expect(() => calculator.subtract(40, {})).to.throw(TypeError);
expect(() => calculator.subtract('40', 2)).to.throw(TypeError);
expect(() => calculator.subtract("40", 2)).to.throw(TypeError);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

expect(() => calculator.subtract([], 2)).to.throw(TypeError);
expect(() => calculator.subtract({}, 2)).to.throw(TypeError);
});

it('should subtract two positive numbers', () => {
it("should subtract two positive numbers", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings must use singlequote.
(learn more)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants