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

Update tests for 'Saddle Points' #766

Closed
wants to merge 1 commit into from
Closed

Update tests for 'Saddle Points' #766

wants to merge 1 commit into from

Conversation

ovidiu141
Copy link
Contributor

@ovidiu141 ovidiu141 commented Oct 16, 2019

also update structure of skeleton file. Fixes: #764

@ovidiu141
Copy link
Contributor Author

The structure was inspired from the typescript version of this exercise.

@SleeplessByte SleeplessByte added the sync 🔄 Sync an exercise with the latest version of the problem-spec label Oct 17, 2019
@SleeplessByte
Copy link
Member

Can't review it right now, but skimming this looks like amazing work @ovidiu141 ! Thank you so far. We'll review shortly.

@ovidiu141
Copy link
Contributor Author

Unfortunately I've deleted my old fork, so now the PR appears to be made from an 'unknown repository'. If this is an issue I can easily recreate the PR from my current fork. Just let my know.

row.forEach((cell, colIndex) => {
if (cell === rowMaxs[rowIndex] && cell === colMins[colIndex]) {
saddlePoints.push([rowIndex, colIndex]);
export class SaddlePoints {
Copy link
Member

Choose a reason for hiding this comment

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

Good that you removed Matrix class & old tests. Here I would suggest using a plain function instead of static method inside class. In #274 we decided to use classes only if some local state is required.

@tejasbubane
Copy link
Member

@ovidiu141 Can you also add version number in package.json just like you did in #784 ?

@tejasbubane
Copy link
Member

Closing in favor of #785

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sync 🔄 Sync an exercise with the latest version of the problem-spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sync Saddle Points with v1.5.0
3 participants