Task with matrix done.#5
Task with matrix done.#5yarikpavlin wants to merge 3 commits intoChangeRequest:masterfrom yarikpavlin:master
Conversation
didva
left a comment
didva
left a comment
There was a problem hiding this comment.
Please go on with other tasks.
| */ | ||
| public class Matrix { | ||
|
|
||
| private final int Rows; // count Rows |
There was a problem hiding this comment.
Please follow java code convention. All fields, methods, method arguments should be named in lowerCamelCase style.
| public Matrix(int[][]data) //Constructor 2d array | ||
| { | ||
| this.Rows = data.length; | ||
| this.Columns = data[0].length; |
There was a problem hiding this comment.
It's possible to create multidimensional arrays in Java like:
int[][] example = new int[3][];
example[0] = new int[100];
example[1] = new int[1000];
example[2] = new int[0];
So determine columns count by the data[0].length is not the best approach. Please add verification that all rows has the same columns number.
By the way, if you will modify this incoming array - your matrix will be changed too. So I'd prefer copying.
|
|
||
| @Override | ||
| public String toString() { | ||
| for(int i = 0; i < Rows;i++) |
There was a problem hiding this comment.
The main point of toString method that it should return string representation of the object and do not print anything.
So please use StringBuilder or any other way to create resulting string and get rid of System.out.print.
| return C; | ||
| } | ||
|
|
||
| public Matrix Calculate(Matrix B,char d) |
There was a problem hiding this comment.
- There is no need in this method at all. Determining operation by
charis not the good approach, because you now which method was called and which operation to execute. So please split your logic to specific methods. - Not sure multiplication is done correctly. Please read about it https://en.wikipedia.org/wiki/Matrix_multiplication
- As you may remember the point was that current matrix should be changed. So no need to create third matrix. You should modify existing one.
|
|
||
| public Matrix Transpose() | ||
| { | ||
| Matrix A = this; |
There was a problem hiding this comment.
Not sure you need extract this to variable. As for me it's even better to use data directly. Like C.data[i][j] = data[j][i];
| int[][]A = new int[2][2];//initialization matrix A | ||
| int[][]B = new int[2][2];//initialization matrix | ||
|
|
||
| A[0][0] = 4; |
There was a problem hiding this comment.
I'd prefer int[][] example = {{1,2}, {3,4}}; such kind of initialization.
| public class TestApp { | ||
|
|
||
| public static void main(String[] args) { | ||
| TestApp app = new TestApp(); |
There was a problem hiding this comment.
Why do you need this object?
didva
left a comment
didva
left a comment
There was a problem hiding this comment.
Please format your code. Please follow java code convention.
| * Created by Yaroslav Pavlinskiy on 06.01.2017. | ||
| */ | ||
| public class Accountant extends Employee { | ||
| Accountant(String name) { |
There was a problem hiding this comment.
I'd prefer public constructors everywhere in this task.
| } | ||
|
|
||
| public double calculateSalaryForEmployee(Employee e) { | ||
| if(e instanceof Programmer || e instanceof Manager || e instanceof Accountant) |
There was a problem hiding this comment.
- Using
instanceofisn't a good practice and should be avoided if it's possible. I do not see any reason for such check here. - The main idea was to calculate salary for a array/list of employees. They should be passed in constructor/setter method. So we'll have some kind of responsibility. This Accountant is responsible for a list/array of employees and calculates their salary.
2 participants
Other tasks will be done soon