HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Finding average of eight immediate neighbors of a matrix

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
matrixaveragefindingeightimmediateneighbors

Problem

I have this basic Java code to find average of eight immediate neighbors of a matrix.

Is there any way to simplify or merge any part of it, or can I refactor it?

I'm a beginner in Java programming and am trying to improve myself.

```
// CODE STSRT
import java.util.Scanner;
import java.lang.*;

class Matrixer
{

double[][] matrix, computedMatrix;
final int rows, cols;

public Matrixer(int N, int M, double[][] imatrix)
{
rows = N;
cols = M;
matrix = imatrix;
computedMatrix = new double[N][M];
}

public void computeAverages()
{
for (int i = 1; i < rows - 1; i++)
{
for (int j = 1; j < cols - 1; j++)
{
computedMatrix[i][j] = cellNeighborsAverage(i, j);
}
}
}

private double cellNeighborsAverage(int row, int col)
{
// Ignore center cell And tack Neighbors
double sum = matrix[row - 1][col - 1] + matrix[row - 1][col]
+ matrix[row - 1][col + 1] + matrix[row][col - 1]
+ matrix[row][col + 1] + matrix[row + 1][col - 1]
+ matrix[row + 1][col] + matrix[row + 1][col + 1];
return sum / 8;
}

public void printComputedMatrix()
{
for (int i = 0; i < rows; i++)
{
for (int j = 0; j < cols; j++)
{
System.out.printf("%.0f", computedMatrix[i][j]);
System.out.print(" ");
}
System.out.println();
}
}

public static void main(String[] args) throws NullPointerException {
//This part for defined the size if matrix
int R,C;
Scanner rc = new Scanner(System.in);
System.out.println("Enter the number of matrix's rows :");
R = rc.nextInt();
Scanner cc = new Scanner(System.in);
System.out.println("Enter the number of matrix's columns :");
C = cc.nextInt();
//End of defined part

// conditio

Solution

Welcome to CodeReview!

Class naming

Maybe it's a term I'm not familiar with but Matrixer does not seem to ring a bell. Is it supposed to identify a "creator of matrixes"? Is it the name of your project? If you haven't made a conscious decision for this name, I would suggest turning it into something "universally" descriptive like MatrixCalculator (pretty much anything that contains the essence of what your class is supposed to achieve).

Indentation

Several places are indented incorrectly/inconsistently. Use a tab (4 spaces) for each new block level.

I would also layout code like your cellNeighborsAverage like this:

double sum = matrix[row - 1][col - 1] + matrix[row - 1][col]
           + matrix[row - 1][col + 1] + matrix[row][col - 1]
           + matrix[row][col + 1] + matrix[row + 1][col - 1]
           + matrix[row + 1][col] + matrix[row + 1][col + 1];


It's just a bit easier to read.

Likewise for your main method: the indentation is all over the place; maybe something went wrong when copy-pasting it here? Keep your if statements in this format:

if(condition) {
    doSomething();
} elseif (anotherCondition) {
    doAnotherThing();
} else {
    doNothing();
}


Parameter naming

Your constructor takes variables N and M in a scientific-oriented context this is an acceptable practice so it will depend on how you view this code but standard conventions state that variables use the lowerCamelCase convention.

I would also make their meaning clearer: N could become rows and M would be columns.

Likewise your parameter is named imatrix. What does that i signify? You can simply call it matrix and use this.matrix to refer to the instance variable inside the constructor body.

I would also distinguish between matrix and computedMatrix. Computed what exactly? Maybe sourceMatrix and averagedMatrix are more appropriate terms?

Method naming

cellNeighborsAverage doesn't really tell me much about what goes on in there. Something about the neighbours average, that's for sure. But what exactly does it do? Does it calculate it? Return it? Do I have to set it?

Conventions state that your method names are built up like [verb][subject] which would become getAverageFromNeighbours.

NullPointerException

The NPE is something you should always prevent and guard against instead of throwing/catching it at runtime. You know the flow of your program and thus you should be able to determine where a NPE could occur. Guard yourself against this by validating parameters, instantiating fields inline when you can, avoid returning null, etc.

Scanners

You only need one Scanner on your System.in which you re-use to get input.

Returning instead of printing

Right now you're working with void methods that print the information directly to the console. This will bring problems when you decide to publish your library (class) online and let people use it. What if that person now wants to write the results to a textfile instead of the console?

Likewise for unit testing this will become troublesome.

Therefore I suggest that you return the results from your methods instead of printing to the console inside Matrixer itself; let the main class decide what happens with your results.

Code Snippets

double sum = matrix[row - 1][col - 1] + matrix[row - 1][col]
           + matrix[row - 1][col + 1] + matrix[row][col - 1]
           + matrix[row][col + 1] + matrix[row + 1][col - 1]
           + matrix[row + 1][col] + matrix[row + 1][col + 1];
if(condition) {
    doSomething();
} elseif (anotherCondition) {
    doAnotherThing();
} else {
    doNothing();
}

Context

StackExchange Code Review Q#48996, answer score: 14

Revisions (0)

No revisions yet.