search

TheAlgorithms / JavaScript

Algorithms and Data Structures implemented in JavaScript for beginners, following best practices.
https://the-algorithms.com/language/javascript
GNU General Public License v3.0
32.53k stars 5.59k forks source link

Consolidating some things #720

Closed defaude closed 3 years ago

defaude commented 3 years ago

Hey everyone! I just arrived here at this repo and would love to contribute a bit :)

Looking at the code organisation and the test setup, it's rather confusing for newcomers right now. Should we discuss those cross-cutting things a bit and then decide the direction going forward? Here's a few ideas - feedback very much welcome:

Ideas

Code structure: Maybe we could define a "template" for code/algorithm files that new code should follow and/or existing code could be migrated towards. Some things to keep in mind, though:

Chores: Automatically running tests and code style checks in the CI is a bit funky right now ;) Mostly due to the fact that there's both Jest and doctest in the mix, of course. However, there's more to do: npm dependencies are outdated, Node.js will have v16 as LTS version starting this October, etc. etc. etc.

Full switch to ESM: Add "type": "module" to our root package.json.

Addressed already:

742

Test setup: There should be just one solution for testing. Do we want to use doctest going forward? Or do we want to use Jest? But keeping both is a source of confusion and will eventually lead to degrading code quality / clarity. (This is basically the attempt to revive the discussion in #142 even though I'd prefer and strongly recommend Jest)

trasherdk commented 3 years ago

Maybe cleaning out unused dependencies from package.json ? As this thing does not have or need a runtime environment, all used dependencies should be in devDependencies ? Maybe adding husky and a pre-commit hook, running standard --fix . could lower the noise ?

defaude commented 3 years ago

That sounds like a nice idea! 👍

Maybe we could even get rid of the job that auto-updates the directory markdown file on a pre-commit basis? (hint, hint: I just improved that one with 2dc8ead...5283a41 but maybe it's even nicer to have it run as a pre-commit hook)

raklaptudirm commented 3 years ago

@cclauss

cclauss commented 3 years ago

An example of this template would be good to see and would spark deeper conversation. I am not sure that idempotency is required for all algorithms. Some of these ideas could enhance the notions expressed in... https://github.com/TheAlgorithms/Javascript/blob/master/CONTRIBUTING.md#what-is-an-algorithm

raklaptudirm commented 3 years ago

Implementation files (modules?) should adhere to a common structure (e.g. ESM

Currently, this repository encourages ESM. If you find files which obey other module systems, please create a list of them to be fixed.

Implementation code should be stateless (i.e. it should be possible to call the implementation multiple times without side effects between test runs)

Again, if you find algorithms with state, create a list in this issue.

Algorithm implementation should refrain from outputting to the console

Same as above.

raklaptudirm commented 3 years ago

Test setup: There should be just one solution for testing. Do we want to use doctest going forward? Or do we want to use Jest? But keeping both is a source of confusion and will eventually lead to degrading code quality / clarity. (This is basically the attempt to revive the discussion in #142 even though I'd prefer and strongly recommend Jest)

We do have testing problems because doctest does not provide a mechanism to ignore files. Removing them may be a good idea.

raklaptudirm commented 3 years ago

We can also add codespell as a github action for checking spellings.

defaude commented 3 years ago

We do have testing problems because doctest does not provide a mechanism to ignore files. Removing them may be a good idea.

So you're saying all tests should be written in Jest? (Which I'd totally vote for)

cclauss commented 3 years ago

I am not a fan of removing tests unless we are replacing them with something equivalent if not better. Let's make sure that we have more tests, not fewer tests.

Issues related to jest... https://github.com/TheAlgorithms/Javascript/search?q=jest&type=issues

raklaptudirm commented 3 years ago

@cclauss We are removing only the doctests, and keeping all the jest ones. How does that sound?

defaude commented 3 years ago

Agreed. Removing tests is the wrong thing to do. We should migrate doctests to Jest instead!

raklaptudirm commented 3 years ago

So, we should migrate everything to jest? Any issues with that?

defaude commented 3 years ago

Regarding the test setup: See #742 - we'll make a list of all doctests, migrate them to Jest and then remove doctest altogether.

defaude commented 3 years ago

Added the "full switch to ESM" part to the description above. This will tell node that EVERY js file is supposed to be an ESM, which will (most likely) break some older files which need to be refactored, of course.

That should be a nice doable chore for someone interested ;) I'm super busy in the next few days, sadly...

raklaptudirm commented 3 years ago

I will try to get some progress on tracking the non ESM files.

raklaptudirm commented 3 years ago

require and module.exports found in:

-----------------------------------------------------------------------------------------------------------------
                 File                    |     Line    |                  Problem
-----------------------------------------------------------------------------------------------------------------
./Maths/PermutationAndCombination.js:    |      57     | const funcs = require("./PermutationAndCombination.js")
./Project-Euler/Problem1.js:             |       7     | const readline = require('readline')
./Conversions/DateDayDifference.js:      |      41     | module.exports = DateDayDifference
./Conversions/DateToDay.js:              |      64     | module.exports = DateToDay
./Conversions/LowerCaseConversion.js:    |      35     | module.exports = LowerCaseConversion
./Conversions/RailwayTimeConversion.js:  |      35     | module.exports = RailwayTimeConversion
./Hashes/SHA1.js:                        |     177     | module.exports = SHA1
./Hashes/SHA256.js:                      |     188     | module.exports = SHA256
./Maths/CheckKishnamurthyNumber.js:      |      44     | module.exports = CheckKishnamurthyNumber
./Maths/CoPrimeCheck.js:                 |      40     | module.exports = CoPrimeCheck
./Maths/GetEuclidGCD.js:                 |      32     | module.exports = GetEuclidGCD
./Maths/PermutationAndCombination.js:    |      50     | module.exports.factorial = factorial
./Maths/PermutationAndCombination.js:    |      51     | module.exports.permutation = permutation
./Maths/PermutationAndCombination.js:    |      52     | module.exports.combination = combination
./Maths/ReverseNumber.js:                |      29     | module.exports = ReverseNumber
./String/AlternativeStringArrange.js:    |      44     | module.exports = AlternativeStringArrange
./String/CheckKebabCase.js:              |      20     | module.exports = CheckKebabCase
./String/CheckPascalCase.js:             |      20     | module.exports = CheckPascalCase
-----------------------------------------------------------------------------------------------------------------
raklaptudirm commented 3 years ago

A List of algorithms that output to the console (the list is too long for me to format sadly):

./Backtracking/AllCombinationsOfSizeK.js:33:      console.log(this.combinationArray)
./Backtracking/GeneratePermutations.js:22:    console.log(arr.join(' '))
./Backtracking/KnightTour.js:62:      console.log(string)
./Backtracking/KnightTour.js:71:  console.log('\n')
./Backtracking/NQueen.js:56:    console.log('\n')
./Backtracking/NQueen.js:58:      console.log(...row)
./Backtracking/Sudoku.js:66:      if (i % 3 === 0 && i !== 0) console.log('- - - - - - - - - - - -')
./Backtracking/Sudoku.js:67:      console.log(
./Bit-Manipulation/BinaryCountSetBits.js:17:console.log(BinaryCountSetBits(251))
./Cache/LFUCache.js:112:  console.log(cache.get(1))
./Cache/LFUCache.js:116:  console.log(cache.get(2)) // cache miss
./Cache/LFUCache.js:120:  console.log(cache.get(1)) // cache miss
./Cache/LFUCache.js:121:  console.log(cache.get(3))
./Cache/LFUCache.js:122:  console.log(cache.get(4))
./Cache/LFUCache.js:124:  console.log('Example Cache: ', cache.cacheInfo(), '\n')
./Cache/LFUCache.js:140:  console.log('Fibonacci Series Cache: ', fibCache.cacheInfo(), '\n')
./Cache/LRUCache.js:95:  console.log(cache.get(1))
./Cache/LRUCache.js:99:  console.log(cache.get(2)) // cache miss
./Cache/LRUCache.js:103:  console.log(cache.get(1)) // cache miss
./Cache/LRUCache.js:104:  console.log(cache.get(3))
./Cache/LRUCache.js:105:  console.log(cache.get(4))
./Cache/LRUCache.js:107:  console.log('Example Cache: ', cache.cacheInfo(), '\n')
./Cache/LRUCache.js:123:  console.log('Fibonacci Series Cache: ', fibCache.cacheInfo(), '\n')
./Ciphers/Atbash.js:29:console.log(decryptedString) // SVOOL DLIOW
./Ciphers/CaesarsCipher.js:36:console.log(decryptedString) // Hello World
./Ciphers/KeyFinder.js:48:        // console.log( k + outStrElement + wordBank[i] );//debug
./Ciphers/KeyFinder.js:148:console.log('Testing: ' + keyFinder('test')) // returns 0
./Ciphers/KeywordShiftedAlphabet.js:71:console.log(encrypt('keyword', 'Hello world!')) // Prints 'Aoggj ujngw!'
./Ciphers/KeywordShiftedAlphabet.js:72:console.log(decrypt('keyword', 'Aoggj ujngw!')) // Prints 'Hello world!
./Ciphers/ROT13.js:16:  console.log(`Original Text = "${messageToBeEncrypted}"`)
./Ciphers/ROT13.js:18:  console.log(`Ciphered Text = "${rot13CipheredText}"`)
./Ciphers/ROT13.js:20:  console.log(`Deciphered Text = "${rot13DecipheredText}"`)
./Ciphers/VigenereCipher.js:75:console.log(messageEncrypt) // "Jhpnr Yrvng!"
./Ciphers/VigenereCipher.js:78:console.log(messageDecrypt) // "Hello World!"
./Ciphers/XORCipher.js:24:console.log('Encrypted: ', encryptedString)
./Ciphers/XORCipher.js:26:console.log('Decrypted: ', decryptedString)
./Conversions/ArbitraryBase.js:42:  console.log(convertArbitraryBase('98', '0123456789', '01234567'))
./Conversions/ArbitraryBase.js:43:  console.log(convertArbitraryBase('98', '0123456789', 'abcdefgh'))
./Conversions/ArbitraryBase.js:44:  console.log(convertArbitraryBase('129', '0123456789', '01234567'))
./Conversions/BinaryToDecimal.js:7:  console.log(`Decimal of ${binaryString} is ${decimalNumber}`)
./Conversions/DecimalToBinary.js:7:  console.log('The decimal in binary is ' + bin.join(''))
./Conversions/DecimalToOctal.js:9:  console.log('The decimal in octal is ' + oct)
./Conversions/HexToDecimal.js:26:console.log(hexToDecimal('5DE9A')) // 384666
./Conversions/HexToDecimal.js:27:console.log(hexToDecimal('3D')) // 61
./Conversions/HexToRGB.js:14:console.log(hexStringToRGB('ffffff'))
./Conversions/OctToDecimal.js:14:console.log(octalToDecimal(56) === 46)
./Conversions/OctToDecimal.js:15:console.log(octalToDecimal(2365) === 1269)
./Conversions/RGBToHex.js:15:console.log(RGBToHex(255, 255, 255) === '#ffffff')
./Conversions/RGBToHex.js:16:console.log(RGBToHex(255, 99, 71) === '#ff6347')
./Conversions/RomanToDecimal.js:36:console.log(romanToDecimal('XXIIVV'))
./Conversions/RomanToDecimal.js:37:console.log(romanToDecimal('MDCCCIV'))
./Conversions/RomanToDecimal.js:38:console.log(romanToDecimal('XXIVI'))
./Dynamic-Programming/ClimbingStairs.js:22:  console.log('Number of ways to climb ' + number + ' stairs in ' + climbStairs(number))
./Dynamic-Programming/EditDistance.js:55:  console.log(minimumEditDistance('horse', 'ros'))
./Dynamic-Programming/EditDistance.js:56:  console.log(minimumEditDistance('cat', 'cut'))
./Dynamic-Programming/EditDistance.js:57:  console.log(minimumEditDistance('', 'abc'))
./Dynamic-Programming/EditDistance.js:58:  console.log(minimumEditDistance('google', 'glgool'))
./Dynamic-Programming/FibonacciNumber.js:17:  console.log(number + 'th Fibonacci number is ' + fibonacci(number))
./Dynamic-Programming/FindMonthCalendar.js:17:    console.log('M   T   W   Th  F   S   Su')
./Dynamic-Programming/FindMonthCalendar.js:33:      console.log(row)
./Dynamic-Programming/LevenshteinDistance.js:45:  console.log('Levenshtein distance between ' + x + ' and ' + y + ' is: ')
./Dynamic-Programming/LevenshteinDistance.js:46:  console.log(calculate(x, y))
./Dynamic-Programming/LongestCommonSubsequence.js:30:  console.log(res)
./Dynamic-Programming/LongestIncreasingSubsequence.js:24:  console.log('Length of Longest Increasing Subsequence is:', res)
./Dynamic-Programming/MaxNonAdjacentSum.js:23:  console.log(maximumNonAdjacentSum([1, 2, 3]))
./Dynamic-Programming/MaxNonAdjacentSum.js:24:  console.log(maximumNonAdjacentSum([1, 5, 3, 7, 2, 2, 6]))
./Dynamic-Programming/MaxNonAdjacentSum.js:25:  console.log(maximumNonAdjacentSum([-1, -5, -3, -7, -2, -2, -6]))
./Dynamic-Programming/MaxNonAdjacentSum.js:26:  console.log(maximumNonAdjacentSum([499, 500, -3, -7, -2, -2, -6]))
./Dynamic-Programming/MinimumCostPath.js:30:  console.log(
./Dynamic-Programming/MinimumCostPath.js:37:  console.log(
./Dynamic-Programming/NumberOfSubsetEqualToGivenSum.js:30:  console.log(result)
./Dynamic-Programming/Shuf.js:94:  console.log(result)
./Dynamic-Programming/SieveOfEratosthenes.js:26:      console.log(i)
./Dynamic-Programming/SudokuSolver.js:48:    console.log(_board)
./Dynamic-Programming/ZeroOneKnapsack.js:65:    console.log(result)
./Geometry/ConvexHullGraham.js:30:    console.log('Minimum of 3 points is required to form closed polygon!')
./Geometry/ConvexHullGraham.js:68:  console.log('The Convex Hull found is: \n')
./Geometry/ConvexHullGraham.js:69:  console.log(hull)
./Graphs/ConnectedComponents.js:53:  console.log(graph.connectedComponents())
./Graphs/Density.js:11:console.log(density(10, 2))
./Graphs/DepthFirstSearchIterative.js:47:  console.log(graph.DFSIterative(5, 1))
./Graphs/DepthFirstSearchIterative.js:48:  console.log(graph.DFSIterative(5, 100))
./Graphs/DepthFirstSearchRecursive.js:42:  console.log(graph.DFSRecursive(5, 1))
./Graphs/DepthFirstSearchRecursive.js:43:  console.log(graph.DFSRecursive(5, 100))
./Graphs/Dijkstra.js:76:console.log(distances)
./Graphs/DijkstraSmallestPath.js:89:console.log("From '" + start + "' to")
./Graphs/DijkstraSmallestPath.js:93:  console.log(' -> ' + s + ': [' + solutions[s].join(', ') + ']   (dist:' + solutions[s].dist + ')')
./Graphs/FloydWarshall.js:39:  console.log(FloydWarshall(
./Graphs/KruskalMST.js:111:  console.log(graph)
./Graphs/KruskalMST.js:112:  console.log(graph.KruskalMST())
./Graphs/NodeNeighbors.js:39:  console.log(graph.nodeNeighbors(1))
./Graphs/NumberOfIslands.js:86:console.log(islands(grid))
./Graphs/PrimMST.js:207:  console.log(graph.PrimMST(1))
./Hashes/SHA1.js:173:console.log(SHA1('A Test'))
./Hashes/SHA1.js:174:console.log(SHA1('A Test'))
./Maths/decimalIsolate.js:13:console.log(decimalIsolate(35.345))
./Maths/decimalIsolate.js:14:console.log(decimalIsolate(56.879))
./Maths/decimalIsolate.js:15:console.log(decimalIsolate(89.5643))
./Maths/decimalIsolate.js:16:console.log(decimalIsolate(38.00))
./Maths/decimalIsolate.js:17:console.log(decimalIsolate(33))
./Maths/IsDivisible.js:13:console.log(isDivisible(10, 5)) // returns true
./Maths/IsDivisible.js:14:console.log(isDivisible(123498175, 5)) // returns true
./Maths/IsDivisible.js:15:console.log(isDivisible(99, 5)) // returns false
./Maths/isOdd.js:12:console.log(isOdd(2))
./Maths/isOdd.js:13:console.log(isOdd(3))
./Maths/MatrixExponentiationRecursive.js:72:  console.log(MatrixExponentiationRecursive(mat, 0))
./Maths/MatrixExponentiationRecursive.js:75:  console.log(MatrixExponentiationRecursive(mat, 1))
./Maths/MatrixExponentiationRecursive.js:78:  console.log(MatrixExponentiationRecursive(mat, 2))
./Maths/MatrixExponentiationRecursive.js:81:  console.log(MatrixExponentiationRecursive(mat, 5))
./Maths/MatrixMultiplication.js:13:      console.log('The columns in this array are not equal')
./Maths/MatrixMultiplication.js:24:    console.log('These matrices do not have a common side')
./Maths/MatrixMultiplication.js:79:console.log(matrixMult(firstMatrix, secondMatrix)) // [ [ 19, 22 ], [ 43, 50 ] ]
./Maths/MatrixMultiplication.js:91:console.log(matrixMult(thirdMatrix, fourthMatrix)) // [ [ 21, 16 ], [ -10, -28 ] ]
./Maths/PermutationAndCombination.js:59:   console.log(funcs.factorial(5));
./Maths/PermutationAndCombination.js:60:   console.log(funcs.permutation(5, 2));
./Maths/PermutationAndCombination.js:61:   console.log(funcs.combination(5, 2));
./Maths/WhileLoopFactorial.js:18:console.log(factorialize(5))
./Maths/WhileLoopFactorial.js:19:console.log(factorialize(4))
./Project-Euler/Problem013.js:119:console.log(findFirstTenDigitsOfSum())
./Project-Euler/Problem014.js:47:console.log(findLongestCollatzSequence())
./Project-Euler/Problem015.js:17:console.log(latticePath(20)) // output = 137846528820
./Project-Euler/Problem020.js:21:console.log(findFactorialDigitSum(100))
./Project-Euler/Problem1.js:25:  console.log(multiplesThreeAndFive(num)) // multiples3_5 function to calculate the sum of multiples of 3 and 5 within num
./Project-Euler/Problem2.js:13:console.log(EvenFibonacci(4e6)) // Sum of even Fibonacci upto 4 Million
./Project-Euler/Problem3.js:20:console.log(largestPrime(problem))
./Project-Euler/Problem4.js:46:console.log(largestPalindromic(3))
./Project-Euler/Problem5.js:22:console.log(findSmallestMultiple())
./Project-Euler/Problem6.js:15:console.log(squareDifference(num))
./Project-Euler/Problem7.js:31:console.log(calculatePrime(num))
./Project-Euler/Problem9.js:27:console.log(findSpecialPythagoreanTriplet())
./Recursive/BinaryEquivalent.js:26:console.log(ans)
./Recursive/BinarySearch.js:23:  console.log('Number Present with odd array length: 5 = ', BinarySearch([1, 2, 3, 4, 5, 6, 7], 5))
./Recursive/BinarySearch.js:24:  console.log('Number Present with even array length: 5 = ', BinarySearch([1, 2, 4, 5, 6], 5))
./Recursive/BinarySearch.js:25:  console.log('Number Present with only single element: 5 = ', BinarySearch([5], 5))
./Recursive/BinarySearch.js:26:  console.log('Number Not Present: 0 = ', BinarySearch([1, 2, 3, 4, 5], 0))
./Recursive/BinarySearch.js:27:  console.log('Undefined number search query = ', BinarySearch([1, 2, 3, 4, 5]))
./Recursive/BinarySearch.js:28:  console.log('With Empty array = ', BinarySearch([], 1))
./Recursive/EucledianGCD.js:33:  console.log('Recursive GCD for %d and %d is %d', first, second, euclideanGCDRecursive(first, second))
./Recursive/EucledianGCD.js:34:  console.log('Iterative GCD for %d and %d is %d', first, second, euclideanGCDIterative(first, second))
./Recursive/factorial.js:14:console.log(factorial(4))
./Recursive/factorial.js:15:console.log(factorial(15))
./Recursive/factorial.js:16:console.log(factorial(0))
./Recursive/FibonacciNumberRecursive.js:12:  console.log(number + 'th Fibonacci number is ' + fibonacci(number))
./Recursive/Palindrome.js:25:  console.log('Palindrome: String: a = ', Palindrome('a'))
./Recursive/Palindrome.js:26:  console.log('Palindrome: String: abba = ', Palindrome('abba'))
./Recursive/Palindrome.js:27:  console.log('Palindrome: String: ababa = ', Palindrome('ababa'))
./Recursive/Palindrome.js:28:  console.log('Not Palindrome: String: abbxa = ', Palindrome('abbxa'))
./Recursive/Palindrome.js:29:  console.log('Not Palindrome: String: abxa = ', Palindrome('abxa'))
./Recursive/SubsequenceRecursive.js:24:    console.log(seq)
./Recursive/TowerOfHanoi.js:6:    console.log(`Move disk 1 from rod ${fromRod} to rod ${toRod}`)
./Recursive/TowerOfHanoi.js:10:  console.log(`Move disk ${n} from rod ${fromRod} to rod ${toRod}`)
./Search/BinarySearch.js:84:console.log(binarySearchRecursive(arr, 3))
./Search/BinarySearch.js:85:console.log(binarySearchIterative(arr, 7))
./Search/BinarySearch.js:86:console.log(binarySearchRecursive(arr, 13))
./Search/BinarySearch.js:88:console.log(binarySearchIterative(stringArr, 'Charlie'))
./Search/BinarySearch.js:89:console.log(binarySearchRecursive(stringArr, 'Zulu'))
./Search/BinarySearch.js:90:console.log(binarySearchIterative(stringArr, 'Sierra'))
./Search/ExponentialSearch.js:55:  console.log('Element not found')
./Search/ExponentialSearch.js:57:  console.log('Element found at position :' + result)
./Search/FibonacciSearch.js:77:console.log('Element found at index:', fibFinder)
./Search/InterpolationSearch.js:43:console.log('Found at position :' + interpolationSearch(arr, 2))
./Search/InterpolationSearch.js:44:console.log('Found at position :' + interpolationSearch(arr, 12))
./Search/InterpolationSearch.js:45:console.log('Found at position :' + interpolationSearch(arr, 1000))
./Search/InterpolationSearch.js:46:console.log('Found at position :' + interpolationSearch(arr, 39))
./Search/LinearSearch.js:10:    console.log('The element was found at ' + (position + 1))
./Search/LinearSearch.js:12:    console.log('The element not found')
./Search/QuickSelectSearch.js:53:console.log(quickSelectSearch(arr, 5)) // [ 19, 21, 28, 41, 5, 66, 333, 11110, 1121111, 7777 ]
./Search/QuickSelectSearch.js:54:console.log(quickSelectSearch(arr, 2)) // [ 19, 5, 21, 41, 28, 333, 11110, 1121111, 7777, 66 ]
./Search/QuickSelectSearch.js:55:console.log(quickSelectSearch(arr, 7)) // [ 19, 5, 21, 41, 28, 66, 333, 7777, 11110, 1121111 ]
./Search/StringSearch.js:83:console.log(stringSearch('Hello search the position of me', 'pos'))
./Sorts/CountingSort.js:38:console.log('\n- Before Sort | Implementation of Counting Sort -')
./Sorts/CountingSort.js:39:console.log(array)
./Sorts/CountingSort.js:41:console.log('- After Sort | Implementation of Counting Sort -')
./Sorts/CountingSort.js:42:console.log(countingSort(array, 0, 5))
./Sorts/CountingSort.js:43:console.log('\n')
./Sorts/FlashSort.js:85:console.log('\n- Before Sort | Implementation of Flash Sort -')
./Sorts/FlashSort.js:86:console.log(array)
./Sorts/FlashSort.js:88:console.log('- After Sort | Implementation of Flash Sort -')
./Sorts/FlashSort.js:89:console.log(flashSort(array))
./Sorts/FlashSort.js:90:console.log('\n')
./Sorts/GnomeSort.js:28:console.log(ar)
./Sorts/GnomeSort.js:31:console.log(ar)
./Sorts/HeapSort.js:55:console.log(ar)
./Sorts/HeapSort.js:58:console.log(ar)
./Sorts/HeapSortV2.js:45:console.log(arr)
./Sorts/InsertionSort.js:25:console.log(arr)
./Sorts/IntroSort.js:271:    console.log('WRONG!!')
./Sorts/IntroSort.js:273:    console.log('RIGHT:)')
./Sorts/IntroSort.js:303:    console.log('WRONG Implented Comparator!!')
./Sorts/IntroSort.js:305:    console.log('Comparator Works Fine:)')
./Sorts/OddEvenSort.js:37:console.log(testArray)
./Sorts/OddEvenSort.js:40:console.log(testArray)
./Sorts/PigeonHoleSort.js:17:  console.log(max)
./Sorts/PigeonHoleSort.js:18:  console.log(min)
./Sorts/PigeonHoleSort.js:38:console.log(arr)
./Sorts/RadixSort.js:49:console.log(ar)
./Sorts/RadixSort.js:52:console.log(ar)
./Sorts/SelectionSort.js:42:  console.log(array)
./Sorts/SelectionSort.js:45:  console.log(array)
./Sorts/ShellSort.js:33:console.log(ar)
./Sorts/ShellSort.js:36:console.log(ar)
./Sorts/TimSort.js:106:    console.log('RIGHT')
./Sorts/TimSort.js:108:    console.log('FAULTY')
./Sorts/TopologicalSort.js:59:console.log(topoSorter.sortAndGetOrderedItems())
./Sorts/WiggleSort.js:23:console.log(arr) // [3, 5, 2, 1, 6, 4]
./Sorts/WiggleSort.js:27:console.log(arr) // [ 3, 5, 2, 6, 1, 4 ]
./String/CheckRearrangePalindrome.js:30:console.log(palindromeRearranging('aaeccrr')) // true
./String/CheckRearrangePalindrome.js:31:console.log(palindromeRearranging('leve')) // false
./String/DiceCoefficient.js:47:  console.log('Dice coefficient of', stringA, 'and', stringB, 'is', dice)
./String/GenerateGUID.js:17:console.log(Guid()) // 'edc848db-3478-1760-8b55-7986003d895f'
./String/LevenshteinDistance.js:40:  console.log(
./Timing-Functions/IntervalTimer.js:85:  console.log(timer.getElapsedTime(initOffset))
./Timing-Functions/IntervalTimer.js:90:  console.log(timer.resetTimer())
./Trees/BreadthFirstTreeTraversal.js:60:console.log(binaryTree.breadthFirst())
lvlte commented 3 years ago

Maybe we should also add some guidelines about file naming convention. For example in Project-Euler directory, we got :

I suggest the following strict naming convention : ProblemXXX.js where is XXX is the problem number, left-padded with zeros, so that the lexicographic order fits the problem numbering order :

lvlte commented 3 years ago

Removing (some of) these logs is not as trivial as it sounds, for example the alogrithm for AllCombinationsOfSizeK.js does not return anything but log combinations as they are found (recursively), so we can't just replace console.log by a return statement but need to revise the implementation somehow.

Also, some logs give useful hints about how to implement the algorithm, and for those not being covered by Jest tests, it might be the only hint a developer/student can find at first glance.

For these reasons, log removal is probaly a more tedious task than it seems, so I suggest creating separated issues for covering :

  1. The conversion of all live code into Jest tests (removing code chunks living outside a module).
  2. Revise all algorithms that log their results instead of returning them (removing logs inherent to a module + fix that module if necessary).

I'm new and would be happy to contrbute, so feel free to assign some tasks to me !

raklaptudirm commented 3 years ago

@lvlte You can open a new pr and start implementing the changes now.

lvlte commented 3 years ago

Ok, I will start working on this tomorrow. Thanks

defaude commented 3 years ago

I'd argue that it should be quite easy to remove console logging from most implementations and simply return results.

For the few cases where the intermediate hits are part of the overall result, there are ways, too: Either collect all intermediates in a variable and return that once recursion finishes up or provide some kind of "sink" callback that gets called with each intermediate value. This "sink" can be passed in, too. Which means in a test, we could just pass in a jest mock function and check the values that got thrown into it.

Example:

// algorithm. counts up to (not including) max, calling sink with each value
function countUpTo(max, sink = value => console.log(value)) {
    for (let i = 0; i < max; ++i) {
        sink(i);
    }
}

// test
describe('countUpTo', () => {
    it('should call the sink fn with the correct values', () => {
        const sink = jest.fn();
        countUpTo(3, sink);
        expect(sink.mock.calls.length).toBe(3);
        expect(sink.mock.calls[0][0]).toBe(0);
        expect(sink.mock.calls[1][0]).toBe(1);
        expect(sink.mock.calls[2][0]).toBe(2);
    });
});

If someone imported this function e.g. in a manual test and simply only did countUpTo(3), the default parameter would kick in and the values will get printed to the console.

defaude commented 3 years ago

... but of course that's always a bit tricky: find out where / how to pluck stuff like this into an existing algorithm ;)

lvlte commented 3 years ago

Hi there,

This issue should be fixed by PR #768 which is ready to merge since 2 days (I can't remove the in progress label). @raklaptudirm is asked for review but there are a lot of (small) changes so do not hesitate to throw an eye @cclauss @defaude.

Thank you by the way @defaude for the recommandations you made on this, it definitely helped.

defaude commented 3 years ago

Sorry for being so quite. Life happened... ;) I'll take a look right away.