1
\$\begingroup\$

I am trying to implement a machine learning algorithm (k-nn for example). But before I attempt to do that I was hoping for some feedback on my current Main class, which essentially builds 8×8-pixel matrices into an array to be manipulated later. (See the data description and sample dataset.)

import java.util.*;
import java.io.*;

public class Main {

    static class Data {

        int[] dataSet;
        int classCode;

        public Data(int[] dataSet, int label) {
            this.dataSet = dataSet;
            this.classCode = label;
        }

        @Override
        public String toString() {
            return "Class Code: " + classCode + " DataSet:" + Arrays.toString(dataSet) + "\n";
        }

    }

    ArrayList<Data> dataSetList;
    int[][] dataArray = new int[2810][65];

    private void readFile(String csvFile) {
        int instances = 0;
        dataSetList = new ArrayList<>();
        try {
            Scanner scan = new Scanner(new BufferedReader(new FileReader(csvFile)));
            while (scan.hasNext()) {
                String line = scan.next();
                String[] extractedDataFromFile = line.split(",");
                for (int i = 0; i < extractedDataFromFile.length; i++) {
                    dataArray[instances][i] = Integer.parseInt(extractedDataFromFile[i]);
                }
                dataSetList.add(new Data(dataArray[instances], dataArray[instances][extractedDataFromFile.length - 1]));
                instances++;
            }
            System.out.println(dataSetList.toString());
        } catch (FileNotFoundException ex) {
            System.out.println(ex.getMessage());
        }
    }

    public static void main(String[] args) {
        Main main = new Main();
        main.readFile("dataset1.csv");
    }

}

This is my first time experimenting with machine learning and any feedback on my current approach as well as an algorithm to look at beyond this would be hugely appreciated.

\$\endgroup\$
1
\$\begingroup\$

Outline

Data and Main aren't exactly helpful class names. I suggest ImageMatrix and ImageMatrixDB.

In some sense, your code is incomplete, because it just prints the data that it read. To make this code useful, you would have to offer a way to retrieve the data — and not just by directly accessing dataSetList and dataArray, which should have been private. I suggest the an outline like this:

ImageMatrix.java

public class ImageMatrix {
    public ImageMatrix(int[] data, int classCode) {
        assert data.length == 64;
        …
    }

    public int[] getData() {
        …
    }

    public int getClassCode() {
        …
    }
}

ImageMatrixDB.java

public class ImageMatrixDB implements Iterable<ImageMatrix> {
    private List<ImageMatrix> list = new ArrayList<>;

    public static ImageMatrixDB load(File f) throws IOException {
        …
    }

    public Iterator<ImageMatrix> iterator() {
        return this.list.iterator();
    }
}

Why are you storing the same data twice, in both dataSetList and dataArray? You should drop the dataArray, which relies on the magic number 2810.

The file parameter should be a File rather than a String, so as not to give the impression that you it accepts the contents of the file rather than the name of the file.

The parsing routine needs a way to report failure to its caller. No, printing an error message doesn't count, as the caller would still proceed obliviously.

Implementation

Why are you using a Scanner? The BufferedReader already has a readLine() method that does what you want. Note that you fail to close the FileReader, BufferedReader, and the Scanner.

In the Data object, the dataSet array has 65 elements: the 64 data points and the class code. The class code has no business being part of that array.

public static ImageMatrixDB load(File f) throws IOException {
    ImageMatrixDB result = new ImageMatrixDB();
    try (FileReader fr = new FileReader(f);
         BufferedReader br = new BufferedReader(fr)) {
        for (String line; null != (line = br.readLine()); ) {
            int lastComma = line.lastIndexOf(',');
            int classCode = Integer.parseInt(line.substring(1 + lastComma));
            int[] data = Arrays.stream(line.substring(0, lastComma).split(","))
                               .mapToInt(Integer::parseInt)
                               .toArray();
            result.list.add(new ImageMatrix(data, classCode));
        }
    }
    return result;
}
\$\endgroup\$
5
  • \$\begingroup\$ Thanks for the reply, you have been very helpful thank you :). Could you just clear something up for me please. It all seems to be implemented correctly but I am being thrown "The public type ImageMatrixDB must be defined in its own file" regarding where you define it as a public class. I might just be overlooking something very trivial. Thanks again \$\endgroup\$
    – Ben411916
    Mar 8 '16 at 7:42
  • \$\begingroup\$ It means just that: if ImageMatrixDB is a public class, then it must be defined in a file named ImageMatrixDB.java. (There are alternatives to splitting it into two files, such as making one of the classes non-public, or making one of them an inner class, but they should each stand as a full class, in my opinion.) \$\endgroup\$ Mar 8 '16 at 7:49
  • \$\begingroup\$ Just a follow up question. I am having issues pulling the results from the loaded file, there any chance you could add a quick edit on what my getData function should look like as I think mine is correct but I get no output, leading me to believe there is an issue with the function itself or my "main" \$\endgroup\$
    – Ben411916
    Mar 9 '16 at 1:34
  • \$\begingroup\$ getData() should just be one return statement. \$\endgroup\$ Mar 9 '16 at 2:41
  • \$\begingroup\$ Thats exactly what I have done "return data;" and get classCode return classCode :/ \$\endgroup\$
    – Ben411916
    Mar 9 '16 at 2:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.