Anthony Mclean Assignment

[unrulytony]

No description provided.

[dimitriharding]

Please see the review changes.

[dimitriharding]

There are a few things that were wrong with this submission so I decided to add the solution here and I will also outline what happened, although your test passed.

// Solution
describe('06', () => {
  // Tip: Use setTimeout to delay the execution of marking

  const listOfPapers = [
    {
      subject: 'Maths',
      wasSubmitted: true,
      markPaper: () => {
        return new Promise((resolve, reject) => {
          setTimeout(() => {
            resolve('Maths paper marked');
          }, 2000);
        });
        // add a promise here that resolves after 2 seconds
        // and print "Maths paper marked"
      },
    },
    {
      subject: 'Geology',
      wasSubmitted: true,
      markPaper: () => {
        return new Promise((resolve, reject) => {
          setTimeout(() => {
            resolve('Geology paper marked');
          }, 2000);
        });
        // add a promise here that resolves after 2 seconds
        // and print "Geology paper marked"
      },
    },
    {
      subject: 'Social Studies',
      wasSubmitted: false,
      markPaper: () => {
        return new Promise((resolve, reject) => {
          setTimeout(() => {
            resolve('Social Studies paper marked');
          }, 2000);
        });
        // add a promise here that resolves after 2 seconds
        // and print "Social Studies paper marked"
      },
    },
  ];

  it('Check if a paper was submitted, and if yes, wait for it to be marked', async () => {
    const spyOnLog = vi.spyOn(console, 'log');

    for (const paper of listOfPapers) {
      if (paper.wasSubmitted === true) {
        await paper.markPaper().then((result) => {
            console.log(result)
        })
      }
    }

    expect(spyOnLog).toHaveBeenCalledWith('Maths paper marked');
    expect(spyOnLog).toHaveBeenCalledWith('Geology paper marked');
    expect(spyOnLog).not.toHaveBeenCalledWith('Social Studies paper marked');
    expect(listOfPapers[0].markPaper()).toBeInstanceOf(Promise);
    expect(listOfPapers[1].markPaper()).toBeInstanceOf(Promise);
    expect(listOfPapers[2].markPaper()).toBeInstanceOf(Promise);
  });
});

[dimitriharding]

Needed to return the promise here rather than create a variable for the promise.

Suggested change

	const promise = new Promise((resolve,reject) => {
	return new Promise((resolve,reject) => {

[dimitriharding]

This is what case the test to pass without actually passing.

Since you used the return statement in the for loop, the markPaper function was returned to the test and since there was no error the test passed before getting to the test cases.

Ideally, the promise needed to be resolved using the await keyword, and printing the results to the console.

Suggested change

	return paper.markPaper()
	if (paper.wasSubmitted === true) {
	await paper.markPaper().then((result) => {
	console.log(result)
	})
	}

[dimitriharding]

Since classes are meant to reduce duplicate code, the expectation was that you define the whatIsMyName method on the parent class "Vehicle" and then all child classes would inherit from it, so you wouldn't have to define the same method on all child classes.

[unrulytony]

Good evening, Thanks for the feedback. For the promise I guess I overthink the solution on that one so I will make the adjustments. The classes I was going to add whatIsMyName method to parent class but I was more focused on the passing of the test and not the function of the code. Will also adjust. Should I commit and do pull requests for the new changes?

…

On Mon, Oct 31, 2022 at 12:12 PM Dimitri Harding ***@***.***> wrote: ***@***.**** requested changes on this pull request. Please see the review changes. ------------------------------ In Anthony-Mclean/06/exercise.test.js <#28 (comment)> : > @@ -0,0 +1,76 @@ +describe('06', () => { + + // Tip: Use setTimeout to delay the execution of marking There are a few things that were wrong with this submission so I decided to add the solution here and I will also outline what happened, although your test passed. // Solution describe('06', () => { // Tip: Use setTimeout to delay the execution of marking const listOfPapers = [ { subject: 'Maths', wasSubmitted: true, markPaper: () => { return new Promise((resolve, reject) => { setTimeout(() => { resolve('Maths paper marked'); }, 2000); }); // add a promise here that resolves after 2 seconds // and print "Maths paper marked" }, }, { subject: 'Geology', wasSubmitted: true, markPaper: () => { return new Promise((resolve, reject) => { setTimeout(() => { resolve('Geology paper marked'); }, 2000); }); // add a promise here that resolves after 2 seconds // and print "Geology paper marked" }, }, { subject: 'Social Studies', wasSubmitted: false, markPaper: () => { return new Promise((resolve, reject) => { setTimeout(() => { resolve('Social Studies paper marked'); }, 2000); }); // add a promise here that resolves after 2 seconds // and print "Social Studies paper marked" }, }, ]; it('Check if a paper was submitted, and if yes, wait for it to be marked', async () => { const spyOnLog = vi.spyOn(console, 'log'); for (const paper of listOfPapers) { if (paper.wasSubmitted === true) { await paper.markPaper().then((result) => { console.log(result) }) } } expect(spyOnLog).toHaveBeenCalledWith('Maths paper marked'); expect(spyOnLog).toHaveBeenCalledWith('Geology paper marked'); expect(spyOnLog).not.toHaveBeenCalledWith('Social Studies paper marked'); expect(listOfPapers[0].markPaper()).toBeInstanceOf(Promise); expect(listOfPapers[1].markPaper()).toBeInstanceOf(Promise); expect(listOfPapers[2].markPaper()).toBeInstanceOf(Promise); }); }); ------------------------------ In Anthony-Mclean/06/exercise.test.js <#28 (comment)> : > @@ -0,0 +1,76 @@ +describe('06', () => { + + // Tip: Use setTimeout to delay the execution of marking + + const listOfPapers = [ + { + subject: "Maths", + wasSubmitted: true, + markPaper: () => { + const promise = new Promise((resolve,reject) => { Needed to return the promise here rather than create a variable for the promise. ⬇️ Suggested change - const promise = new Promise((resolve,reject) => { + return new Promise((resolve,reject) => { ------------------------------ In Anthony-Mclean/06/exercise.test.js <#28 (comment)> : > + //promiseMaths.then((value)=>{ + // console.log(value); + //}) + //let results = await promiseMaths + //console.log(results); + // try { + // //for (let i = 0; i< listOfPapers.length; i++){ + // const value = await listOfPapers[i].markPaper() + // //console.log + // console.log(value) + // } catch (error){ + // console.log(error) + // } + for await (const paper of listOfPapers){ + if(paper.wasSubmitted === true){ + return paper.markPaper() This is what case the test to pass without actually passing. Since you used the return statement in the for loop, the markPaper function was returned to the test and since there was no error the test passed before getting to the test cases. Ideally, the promise needed to be resolved using the await keyword, and printing the results to the console. ⬇️ Suggested change - return paper.markPaper() + if (paper.wasSubmitted === true) { + await paper.markPaper().then((result) => { + console.log(result) + }) + } ------------------------------ In Anthony-Mclean/08/exercise.test.js <#28 (comment)> : > + * 1. Create a Vehicle parent + * - wheels as a property + * - bodyType as a property + * + * 2. Create different kind of vehicles from the parent + * - each vehicle should have a color + * - each vehicle should have a method called "whatIsMyName" that return the bodyType of the vehicle + * - Example: "I am a sedan" + */ + + test('Create a Vehicle class, and create different type of vehicles that extends', () => { + class Vehicle { + constructor (wheels,bodyType){ + this.wheels = wheels; + this.bodyType = bodyType; + } Since classes are meant to reduce duplicate code, the expectation was that you define the whatIsMyName method on the parent class "Vehicle" and then all child classes would inherit from it, so you wouldn't have to define the same method on all child classes. — Reply to this email directly, view it on GitHub <#28 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AWV6DXUT2Q5BII6AR33CTNDWGAKZBANCNFSM6AAAAAARRS3TWQ> . You are receiving this because you authored the thread.Message ID: <QualityWorksCG/javascript-and-git-automation-bootcamp/pull/28/review/1162398898 @github.com>