add MinMax game theory JS #1193 commented out test case #1278

devhiteshk · 2023-01-13T07:56:05Z

know more

Describe your change:

[x ] Add an algorithm?
[x ] Fix a bug or typo in an existing algorithm?
[ x] Documentation change?

Checklist:

[x ] I have read CONTRIBUTING.md.
[x ] This pull request is all my own work -- I have not plagiarized.
[x ] I know that pull requests will not be merged if they fail the automated tests.
[ x] This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
[x ] All new JavaScript files are placed inside an existing directory.
[ x] All filenames should use the UpperCamelCase (PascalCase) style. There should be no spaces in filenames.
Example:UserProfile.js is allowed but userprofile.js,Userprofile.js,user-Profile.js,userProfile.js are not
[x ] All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
[x ] If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

devhiteshk · 2023-01-13T07:58:07Z

Dear Maintainers please have a look when you get active :)

Maths/Fibonacci.js

appgurueu

You have various unrelated changes in there, including bumping some dependencies.

devhiteshk · 2023-01-13T09:12:41Z

I think now it is fine

devhiteshk · 2023-01-14T18:01:14Z

You have various unrelated changes in there, including bumping some dependencies.

All conflicts have now been resolved

appgurueu

Consider refactoring out the class. In this example, a simple function should suffice. If you don't want to repeat some calculations, consider a simple function containing a recursive closure:

export default function solveMinMax(scores, ...) {
    const height = Math.log2(scores.length)
    const solve = (...) => {
        ...
        ...(solve(...), solve(...))
        ...
    }
    return solve(...)
}

Furthermore, what exactly is the merit of this? In its current form, this:

Needs to have all scores evaluated beforehand anyways, because they must be provided in an array;
Always recurses into both branches.

Consider implementing lazily evaluating scores. I'd suggest passing a function to get a score for a node rather than an array which needs to be fully evaluated. This can also help in making this more flexible.

appgurueu · 2023-01-14T18:41:07Z

Project-Euler/test/Problem044.test.js

-  test('if the number is greater or equal to 2167', () => {
-    expect(problem44(2167)).toBe(8476206790)
-  })
+  // test('if the number is greater or equal to 2167', () => {


This unrelated change still doesn't belong here.

appgurueu · 2023-01-14T18:42:57Z

Backtracking/tests/MinMax.test.js

+import { MinMax } from '../MinMax'
+
+describe('MinMax', () => {
+  it('should return 65 for MinMax([90, 23, 6, 33, 21, 65, 123, 34423],0,0,true)', () => {


This test is practically equivalent to the below test. You're also duplicating the contents in the description. Please see the TypeScript guide for writing good tests.

appgurueu · 2023-01-14T18:43:24Z

Backtracking/MinMax.js

+  }
+}
+
+// MinMax(scores, depth, nodeindex, ismax)


This outcommented code ought to be removed.

appgurueu · 2023-01-14T18:43:51Z

Backtracking/MinMax.js

+//     >>> minimax(0, 0, True, scores, this.height)
+//     12
+
+class MinMax {


Why not prefix this with export (perhaps even export default) rather than exporting below?

appgurueu · 2023-01-14T18:45:08Z

Backtracking/MinMax.js

+//     12
+
+class MinMax {
+  constructor (scores, depth, nodeIndex, isMax) {


This constructor is completely superfluous - you're passing all these as arguments to solve later on. I'm questioning whether this should be a class at all - IMO it should just be a single solve function that checks it parameters and recurses.

appgurueu · 2023-01-14T18:45:24Z

Backtracking/MinMax.js

+    )
+  }
+
+  get_ans () {


Not in lowerCamelCase

appgurueu · 2023-01-14T18:45:38Z

Backtracking/MinMax.js

+  }
+
+  get_ans () {
+    this.answer = this.solve(


Storing the answer in the class is dirty IMO; why not just return it?

appgurueu · 2023-01-14T18:46:32Z

Backtracking/MinMax.js

+    this.answer = -1
+  }
+
+  solve (depth, nodeIndex, isMax, scores, height) {


You don't have to pass all of these as parameters - you already have them as instance variables.

appgurueu · 2023-01-14T18:59:54Z

Backtracking/MinMax.js

+ *
+ */
+
+//     nodeIndex is index of current node in scores[].


This comment is messy. There are no Python-like doc comments in this repo.

appgurueu · 2023-01-14T19:00:04Z

Backtracking/MinMax.js

@@ -0,0 +1,86 @@
+/*
+ *
+ *  Min Max problem


Please make this a proper JSDoc comment.

devhiteshk · 2023-01-15T04:23:53Z

I think I can't code :(

appgurueu · 2023-01-15T10:31:49Z

Don't worry, your code seems to be a correct port of the code found on Geeks for Geeks. It's just that the Geeks for Geeks code is poorly structured (IMO).

devhiteshk added 6 commits January 13, 2023 12:44

"add MinMax JS and .at() fix ✅295/295"

7c47538

add MinMax JS and .at() fix ✅295/295

814f606

"add MinMax Solution for JS add Url - ✅295/295"

af87b8c

add MinMax Solution for JS spell fix - ✅295/295

f8329bc

"spelling fix L:84 MinMax.js"

eaa4588

add MinMax JS and fixes: #{$1193}

a8049c6

devhiteshk requested review from raklaptudirm and appgurueu as code owners January 13, 2023 07:56

appgurueu reviewed Jan 13, 2023

View reviewed changes

Maths/Fibonacci.js Outdated Show resolved Hide resolved

appgurueu requested changes Jan 13, 2023

View reviewed changes

devhiteshk added 5 commits January 13, 2023 14:29

undo unrelated

947268f

undo unrelated

6b06804

undo unrelated change

a9be8e4

undo package.json

a92e33d

undo package-lock

7ddcd80

devhiteshk changed the title ~~add MinMax game theory JS #1193 commented out test case and .at(index) fix for not passing tests~~ add MinMax game theory JS #1193 commented out test case Jan 13, 2023

devhiteshk requested a review from appgurueu January 13, 2023 09:42

appgurueu requested changes Jan 14, 2023

View reviewed changes

appgurueu reviewed Jan 14, 2023

View reviewed changes

Backtracking/MinMax.js

@@ -0,0 +1,86 @@

/*

*

* Min Max problem

Copy link

Collaborator

appgurueu Jan 14, 2023

Choose a reason for hiding this comment

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

Please make this a proper JSDoc comment.

devhiteshk closed this Jan 15, 2023

Uh oh!

add MinMax game theory JS #1193 commented out test case #1278

add MinMax game theory JS #1193 commented out test case #1278

Uh oh!

Conversation

devhiteshk commented Jan 13, 2023

Describe your change:

Checklist:

Uh oh!

devhiteshk commented Jan 13, 2023

Uh oh!

Uh oh!

appgurueu left a comment

Choose a reason for hiding this comment

Uh oh!

devhiteshk commented Jan 13, 2023

Uh oh!

devhiteshk commented Jan 14, 2023

Uh oh!

appgurueu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devhiteshk commented Jan 15, 2023

Uh oh!

appgurueu commented Jan 15, 2023

Uh oh!

Uh oh!

appgurueu left a comment •

edited

Loading