Added gaussian elimination in js #464

Ariana1729 · 2018-10-06T14:56:56Z

No description provided.

Butt4cak3

Thanks for contributing, as always!

There are quite a few things I need you to change, but nothing major. Mostly style related.

We use 2 spaces as indentation for JavaScript, not 4.
We use camelCase for function and method names in JavaScript.

As a side note: Did you take the C implementation as a reference? Your code looks suspiciously similar to it. It's not wrong or bad. I just noticed. That would also explain the \n in all your console.logs.

Butt4cak3 · 2018-10-06T16:06:29Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

@@ -0,0 +1,112 @@
+function gaussian_elimination(a){


Please add a space between ) and { here and in all other function declarations where you forgot it.

Butt4cak3 · 2018-10-06T16:10:24Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

@@ -0,0 +1,112 @@
+function gaussian_elimination(a){
+ var rows = a.length


var is a pretty outdated keyword in JavaScript. These days, you should really use either let or const to declare a variable. let is an objectively better version of var. const works the same as let except that you can't overwrite a variable declared with const.

The rule of thumb is: Always use const unless you have to overwrite a variable. That would mean that both rows and cols should be declared as const.

ah ok thanks!

Butt4cak3 · 2018-10-06T16:38:08Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+ }
+ }
+
+ if (a[pivot][col] == 0) {


Always use === or !==.

Butt4cak3 · 2018-10-06T16:38:58Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+ continue;
+ }
+
+ if (col != pivot) {


Always use === or !==.

Butt4cak3 · 2018-10-06T16:39:11Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+ }
+
+ if (col != pivot) {
+ let t=a[col];


This should be const as explained further up.

Also, please add spaces between operators and operands like t = a[col]. That goes for the next 2 lines as well.

Butt4cak3 · 2018-10-06T16:48:12Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+ var row = 0;
+
+ for (let col = 0; col < cols - 1; ++col) {
+ if (a[row][col] != 0) {


Always use === or !==.

Butt4cak3 · 2018-10-06T16:50:29Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+ }
+}
+
+var a = [[3, 2 , -4, 3 ],


a should be const.

Butt4cak3 · 2018-10-06T16:52:51Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+
+gaussian_elimination(a);
+console.log("Gaussian elimination:\n");
+for(let i=0;i<a.length;++i){


As I mentioned earlier, please make sure to space things out to make them more readable. for (let i = 0; i < a.length; +) {

That goes for pretty much every line from here on out.

Butt4cak3 · 2018-10-06T17:02:44Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+gaussian_elimination(a);
+console.log("Gaussian elimination:\n");
+for(let i=0;i<a.length;++i){
+ let txt=""


This snippet of code appears 3 times. I suggest creating a function to handle the matrix output. Something like this:

function printMatrixRow(row) { const text = row .map(v => (v < 0 ? " " : " ") + v.toPrecision(8)) .join(""); console.log(text); } function printMatrix(a) { for (const row of a) { printMatrixRow(row); } }

Then you could just printMatrix(a) and printMatrixRow(sol) and you're done.

Butt4cak3 · 2018-10-06T17:04:22Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+ }
+
+ if (a[pivot][col] == 0) {
+ console.log("The matrix is singular.\n");


The trailing \n is not needed. console.log does that for you.

Ariana1729 · 2018-10-06T17:20:51Z

Yeah used the c code as reference+keeping variables mostly the same so when switching between languages it's easier

Butt4cak3 · 2018-10-06T17:29:45Z

Don't forget to update the line numbers in the .md file according to the changes you made.

Ariana1729 · 2018-10-06T17:36:44Z

Hmm the line numbers were the same unless I accidentally newlined somewhere
Works on my local aaa

Butt4cak3 · 2018-10-06T17:39:56Z

Then everything seems to be fine. It was just a reminder, because people often forget that.

I'll take a look at the updated code later and if not, tomorrow afternoon my time (CEST). So far it looks good, though.

Ariana1729 · 2018-10-06T17:41:13Z

Ah ok thanks!

Butt4cak3

Just a couple more very minor things. If you fix these, the PR will be ready to merge. Sorry for being so nitpicky, but we want to make sure that the code examples are as readable as possible.

Butt4cak3 · 2018-10-07T14:35:23Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+ let row = 0;
+
+ for (let col = 0; col < cols - 1; ++col) {
+


This empty line here looks weird. It seems quite random because it's the only for loop that has it. Could you remove it for consistency?

Butt4cak3 · 2018-10-07T14:37:50Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+
+const a = [[3, 2, -4, 3],
+ [ 2, 3, 3, 15],
+ [ 5, -3, 1, 14]];


I suggest you format the matrix like this:

const a = [ [3, 2, -4, 3], [2, 3, 3, 15], [5, -3, 1, 14] ];

Butt4cak3 · 2018-10-07T14:38:36Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+function backSubstitution(a) {
+ const rows = a.length;
+ const cols = a[0].length;
+ const sol=[];


Put spaces around the = here.

Butt4cak3 · 2018-10-07T14:40:11Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+}
+
+function backSubstitution(a) {
+ const rows = a.length;


The rows variable is unused. You can remove this declaration.

Ariana1729 · 2018-10-07T14:53:24Z

Okie I've made the changes

Butt4cak3 · 2018-10-07T15:26:18Z

Great, thanks!

Butt4cak3 · 2018-10-07T15:30:07Z

contents/gaussian_elimination/code/javascript/gaussian_elimination.js

+
+const a = [
+ [3, 2, -4, 3],
+[2, 3, 3, 15],


This line has a tab instead of 2 spaces.

oh oops tab looked like 2 spaces to me thanks

I made a mistake.

Butt4cak3 · 2018-10-08T09:13:39Z

I just went through the code one last time before I could merge and then I realized that I messed up during my review yesterday. You remember how I told you that the rows variable was unused? Well, I put that comment on the wrong line. The rows variable in gaussJordan() is unused, not the one in backSubstitution(). By removing the variable that I told you to remove yesterday, we broke the code. I'm sorry. You'll have to undo that and remove the rows variable from gaussJordan() instead.

We'll get this PR merged today. I promise. :D

Ariana1729 · 2018-10-08T23:45:16Z

whoops died on my bed too early yesterday haha, fixed the code

Butt4cak3

Alright. Finally. :D

Added gaussian elimination in js

1222119

Gathros added Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) Hacktoberfest The label for all Hacktoberfest related things! labels Oct 6, 2018

Butt4cak3 self-assigned this Oct 6, 2018

Butt4cak3 requested changes Oct 6, 2018

View reviewed changes

Updated the js code

5089d26

Removed redundent "\n"s

779feb0

Butt4cak3 requested changes Oct 7, 2018

View reviewed changes

Ariana1729 added 2 commits October 7, 2018 22:46

Cleaned up a little more

0e16c01

Updated line numbers

9119a4e

Butt4cak3 approved these changes Oct 7, 2018

View reviewed changes

Butt4cak3 previously requested changes Oct 7, 2018

View reviewed changes

random tab replaced oops

d16de11

Ariana1729 added 2 commits October 9, 2018 07:42

Fixed code

1cbec61

Fixed line numbers

361174f

Butt4cak3 approved these changes Oct 9, 2018

View reviewed changes

Butt4cak3 merged commit db5b251 into algorithm-archivists:master Oct 9, 2018

Ariana1729 deleted the gausselim_js branch October 11, 2018 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Added gaussian elimination in js #464

Added gaussian elimination in js #464

Uh oh!

Ariana1729 commented Oct 6, 2018

Butt4cak3 left a comment •

edited

Loading

Butt4cak3 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Ariana1729 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Butt4cak3 Oct 6, 2018

Ariana1729 commented Oct 6, 2018

Butt4cak3 commented Oct 6, 2018

Ariana1729 commented Oct 6, 2018

Butt4cak3 commented Oct 6, 2018

Ariana1729 commented Oct 6, 2018

Butt4cak3 left a comment

Butt4cak3 Oct 7, 2018

Butt4cak3 Oct 7, 2018

Butt4cak3 Oct 7, 2018

Butt4cak3 Oct 7, 2018

Ariana1729 commented Oct 7, 2018

Butt4cak3 commented Oct 7, 2018

Butt4cak3 Oct 7, 2018

Ariana1729 Oct 7, 2018

Butt4cak3 commented Oct 8, 2018

Ariana1729 commented Oct 8, 2018

Butt4cak3 left a comment

Labels

3 participants

		@@ -0,0 +1,112 @@
		function gaussian_elimination(a){
		var rows = a.length

Uh oh!

Added gaussian elimination in js #464

Added gaussian elimination in js #464

Uh oh!

Conversation

Ariana1729 commented Oct 6, 2018

Butt4cak3 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ariana1729 commented Oct 6, 2018

Butt4cak3 commented Oct 6, 2018

Ariana1729 commented Oct 6, 2018

Butt4cak3 commented Oct 6, 2018

Ariana1729 commented Oct 6, 2018

Butt4cak3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ariana1729 commented Oct 7, 2018

Butt4cak3 commented Oct 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Butt4cak3 commented Oct 8, 2018

Ariana1729 commented Oct 8, 2018

Butt4cak3 left a comment

Choose a reason for hiding this comment

Labels

3 participants

Butt4cak3 left a comment •

edited

Loading