Skip to content

Added Matrix class#1

Open
Kotybamic wants to merge 1 commit intomainfrom
development
Open

Added Matrix class#1
Kotybamic wants to merge 1 commit intomainfrom
development

Conversation

@Kotybamic
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown
Owner Author

@Kotybamic Kotybamic left a comment

Choose a reason for hiding this comment

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

Took a look at the code and noticed some errors, check them out

Comment thread Matrix.java
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Checked every import using ctrl-f, this import is not used anywhere and is unnecessary clutter.

Comment thread Matrix.java
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.UnsupportedEncodingException;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Similar clutter, this import is not utilized throughout the code

Comment thread Matrix.java
Scanner inFile = new Scanner(new FileReader(filename));

// Initialize array using first line of file -- size parameters
String arrSize = inFile.nextLine();
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

should have some sort of catcher in the case that there is no next line, right now the program will crash if there is no next line

Comment thread Matrix.java
int i = 0;
String rowData;
StringTokenizer st2;
while (inFile.hasNextLine()) {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

theres no cap on this while loop, needs a check for i<m_rows to prevent it from going past the limit

Comment thread Matrix.java
}

public Matrix add(Matrix m) {// add 'this' matrix to matrix m
Matrix newMat = new Matrix(this.m_rows, this.n_cols);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This function does not check the dimensions of the matrixes to make sure they can be added together to begin with

Comment thread Matrix.java
Matrix id = new Matrix(squareSize, squareSize);

for (int i = 0; i < squareSize; i++) {
for (int j = 0; i < squareSize; i++) {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

broken loop, incrementing the wrong variable should be j++

Comment thread Matrix.java
}

public String toString() {
String result = "";
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Not really an error, this repeated concatenation will be inefficient when it comes to larger matrices

Comment thread Matrix.java
}

public Matrix transpose() {
Matrix result = new Matrix(this.m_rows, this.n_cols);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This needs to be swapped, if you transpose a 2 by 3 matrix, the expected output should be 3 by 2

Comment thread Matrix.java
this.arr = new double[m][n];
for (int i = 0; i < this.m_rows; i++) {
for (int j = 0; j < this.n_cols; j++) {
this.arr[i][j] = 0;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

There is absolutely no need for this. In Java, arrays are already initialized to 0.0

Comment thread Matrix.java
return result;
}

public Matrix inverse() {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

this might work for small matrixes, but even scaling a bit will result in a huge increase in processes, use a different method to get to the output we want

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.

1 participant