Skip to content

Dev#1

Open
fManela wants to merge 7 commits into
gemtechd:mainfrom
fManela:dev
Open

Dev#1
fManela wants to merge 7 commits into
gemtechd:mainfrom
fManela:dev

Conversation

@fManela

@fManela fManela commented Jul 13, 2025

Copy link
Copy Markdown

@gemtechd write test on the code

@gemtechd gemtechd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

  1. remove all comments before pushing
  2. each test has to execute only one function
  3. where are the tests for the geometry-calculation.ts file?

Comment thread modules/ecs6-class/line.ts Outdated
// return new Point({ x, y })
// }
// }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What are these comments?

Comment thread test/test-line.test.ts
const p2 = new Point({ x: 3, y: 6 });
const line = new Line({ point1: p1, point2: p2 });
line.calculateSlope();
line.calculateNOfLineFunction();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why did you execute both functions in the test?
You have to execute one function

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

???

Comment thread test/test-geometry.test.ts Outdated
l1.calculateNOfLineFunction();
const l2 = new Line({ point1: new Point({ x: 2, y: 2 }), point2: new Point({ x: 3, y: 3 }) });
l2.calculateSlope();
l2.calculateNOfLineFunction();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what are these functions?
read the readme, there is an option to change the code itself

Comment thread test/test-line.test.ts
const p2 = new Point({ x: 3, y: 6 });
const line = new Line({ point1: p1, point2: p2 });
line.calculateSlope();
line.calculateNOfLineFunction();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

???

@gemtechd gemtechd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

read the comments and fix the code

Comment thread modules/geometry-calculation.ts Outdated
Comment thread test/test-geometry.test.ts Outdated
Comment thread test/test-geometry.test.ts Outdated
const line2 = new Line({ point1: new Point({ x: 0, y: 1 }), point2: new Point({ x: 1, y: 2 }) });
line1.calculateSlope();
line1.calculateNOfLineFunction();
line2.calculateSlope();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

executing more then one function on a test

@fManela

fManela commented Jul 14, 2025 via email

Copy link
Copy Markdown
Author

@fManela

fManela commented Jul 14, 2025 via email

Copy link
Copy Markdown
Author

@fManela fManela requested a review from gemtechd July 15, 2025 11:50
@fManela

fManela commented Jul 15, 2025 via email

Copy link
Copy Markdown
Author

Comment thread modules/geometry-calculation.ts Outdated

export const calculateJunctionPoint = (line1: Line, line2: Line): boolean | Point | undefined => {
if (line1.slope === undefined || line2.slope === undefined || line1.n === undefined || line2.n === undefined) {
return undefined;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

you can calculate the slope and the n of each line
use the functions from the class

@fManela

fManela commented Jul 15, 2025 via email

Copy link
Copy Markdown
Author

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.

2 participants