Is this class design wrong?

Question

I'm starting as a java developer. I'm developing a stock app with java and mysql.

I have big question. I can not figured out where I should place method to save datas. Should I create a new class for DB operatios, or should I place them in inventory class.

Thanks!!

My classes are:

package inventory;

public class Item
{
private String bc; // ITEM BARCODE
private String description; // ITEM DESCRIPTION
private int nItm; // NUMBER OF ITEMS IN A PACK OR ITEMS

/**
 * Constructor for objects of class item
 *  - Initialise instance variables bc, description to null and nItm to 0
 */
public Item()
{
    // initialise instance variables
    bc = null;
    description = null;
    nItm = 0;
}

/**
 * Constructor for objects of class item
 *  - Initialise instance variables to a given parameters.
 *  @param bc           String item codebar.
 *  @param description  String description of the item.
 *  @param nItm         int items or number of items in a pack
 */
public Item(String bc,String description,int nItm)
{
    this.bc = bc;
    this.description = description;
    this.nItm = nItm;
}

/**
 * Gets a codebar from the item
 *
 * @return     A String with the codebar item.
 */
public String getBarcode()
{
    // put your code here
    return bc;
}

/**
 * Gets description from the item
 * 
 * @return     A String with the description item.
 */
public String getDescription()
{
    return description;
}

/**
 * Gets the number of items in stock
 * 
 * @return     Number of items in stock.
 */
public int getNumberOfItems()
{
    return nItm;
}

/**
 * Set the value for codebar
 * 
 * @param  bc  Barcode item. 
 */
public void setBarcode(String bc)
{
    this.bc = bc;
}

/**
 * Set the value for description
 * 
 * @param  description   Description item.
 */
public void setDescription(String description)
{
    this.description = description;
}

/**
 * Set the value for nItm
 * 
 * @param  nItm   Number of items or items in a pack
 */
public void setNumberOfItems(int nItm)
{
    this.nItm = nItm;
}
/**
 * Shows an item with this template "| %-16s | %-44s | %5d |" 
 */
public void showItem()
{
    System.out.print("\f");
    System.out.printf("+------------------+----------------------------------------------+-------+\n");
    System.out.printf("| BARCODE          | DESCRIPTION                                  | STOCK |\n");
    System.out.printf("+------------------+----------------------------------------------+-------+\n");
    System.out.printf("| %-16s | %-44s | %5d |",this.bc,this.description,this.nItm);
    System.out.printf("+-------------------------------------------------------------------------+\n");
}

}

package inventory;
import javax.swing.JTable;
import javax.swing.table.DefaultTableModel;
import DataBases.*;
import java.util.Scanner;
import java.sql.*;
public class Inventory
{
// instance variables - replace the example below with your own
private Item[] inventory; // array of items
public int nreg; // NUMBER OF ITEMS STORED

/**
 * Constructor for objects of class Inventory
 */
public Inventory() throws SQLException,ClassNotFoundException,InstantiationException,IllegalAccessException
{
    // initialise instance variables

    this.inventory = new Item[100];
    loadData();
}

/**
 * An example of a method - replace this comment with your own
 * 
 * @param  y   a sample parameter for a method
 * @return     the sum of x and y 
 */
public void addItem(String bc,String description,int nItm)
{
    // put your code here
    Item item;
    item = new Item(bc,description,nItm);
    inventory[nreg] = item;
    nreg++;
}

/**
 * An example of a method - replace this comment with your own
 * 
 * @param  y   a sample parameter for a method
 * @return     the sum of x and y 
 */
public void loadData() throws SQLException,ClassNotFoundException,InstantiationException,IllegalAccessException
{
    Mysql c = new Mysql();
    ResultSet rs;

    c.On();
    rs = c.ExeGet("SELECT * FROM item");
    while(rs.next())
    {
        inventory[nreg] = new Item(rs.getString("barcode"),rs.getString("description"),rs.getInt("nItems"));
        this.nreg++;
    }
    c.Off();
}

/**
 * An example of a method - replace this comment with your own
 * 
 * @param  y   a sample parameter for a method
 * @return     the sum of x and y 
 */
public void searchItem()
{
    System.out.print("\f");
    int op;
    System.out.printf("\tSEARCH MENU\n");
    System.out.printf("\t===========\n\n");
    System.out.printf("\t1.BY BARCODE.\n");
    System.out.printf("\t2.BY ITEM.\n");
    System.out.printf("\t3.BY NUMBER OF ITEMS.\n");
    System.out.printf("\t0.EXIT.\n\n");
    System.out.printf("\tOPTION: ");

    Scanner reader = new Scanner(System.in);
    op = reader.nextInt();
    switch(op)
    {
        case 0: break;
        //case 1: searchByBarcode();break;
        //case 2: searchByItem();break;
        case 3: searchByNumberOfItems();break;
        default: break;
    }
}

/**
 * An example of a method - replace this comment with your own
 * 
 * @param  y   a sample parameter for a method
 * @return     the sum of x and y 
 */
/*
private void searchByBarcode()
{
    int op;
    int flag = 0;
    Item reg = new Item();

    do{
        System.out.print("\f");
        System.out.println("OPTION 1. OK");
        System.out.printf("BARCODE: ");
        String bc;
        Scanner reader = new Scanner(System.in);
        bc = reader.nextSt();
    //  AQUI VA UN TRY COMO UNA CASA
        for(int i=0;i<nreg;i++)
        {
            if (inventory[i].getBarcode() == bc)
            {
                reg = inventory[i];
                flag = 1;
                break;
            }
        }
        if (flag == 1)
        {
            reg.showItem();
        }
        else
        {
            System.out.printf("I CAN NOT FIND %d",bc);
        }
        System.out.println("CONTINUE SEARCHING (0/1): ");
        op = reader.nextInt();

    }while(op != 1);
}
*/

/**
 * An example of a method - replace this comment with your own
 * 
 * @param  y   a sample parameter for a method
 * @return     the sum of x and y 
 */
private void searchByNumberOfItems()
{
    System.out.print("\f");
    System.out.println("OPTION 3. OK");
}

/**
 * An example of a method - replace this comment with your own
 * 
 * @param  y   a sample parameter for a method
 * @return     the sum of x and y 
 */
public void addNewItem(String barcode,String description,int nItm) throws SQLException,ClassNotFoundException,InstantiationException,IllegalAccessException
{
    Mysql c = new Mysql();
    Item i = new Item(barcode,description,nItm);
    String q = "INSERT INTO inventory.item (barcode,description,nItems) VALUES ('" + i.getBarcode() + "','" 
                + i.getDescription() + "'," + i.getNumberOfItems() + ");";

    c.On();
    c.Exe(q);
    c.Off();
    nreg++;
}

/**
 * An example of a method - replace this comment with your own
 * 
 * @param  y   a sample parameter for a method
 * @return     the sum of x and y 
 */
public void listItems() throws SQLException,ClassNotFoundException,InstantiationException,IllegalAccessException
{
    Mysql c = new Mysql();
    ResultSet rs;

    c.On();
    rs = c.ExeGet("SELECT * FROM item");
    System.out.print("\f");
    System.out.printf("+------------------+----------------------------------------------+-------+\n");
    System.out.printf("| BARCODE          | DESCRIPTION                                  | STOCK |\n");
    System.out.printf("+------------------+----------------------------------------------+-------+\n");
    while(rs.next())
    {
        //System.out.println("" + rs.getString("barcode") + "     " + rs.getString("description") + "     " + rs.getInt("nItems"));

        System.out.printf("| %-16s | %-44s | %5d |\n",rs.getString("barcode"),rs.getString("description"),rs.getInt("nItems"));
    }
    System.out.printf("+-------------------------------------------------------------------------+\n");
    c.Off();
}

public void listArray()
{
    System.out.print("\f");
    System.out.printf("+------------------+----------------------------------------------+-------+\n");
    System.out.printf("| BARCODE          | DESCRIPTION                                  | STOCK |\n");
    System.out.printf("+------------------+----------------------------------------------+-------+\n");
    for(int i = 0;i<nreg;i++)
    {
        System.out.printf("| %-16s | %-44s | %5d |\n",inventory[i].getBarcode(),inventory[i].getDescription(),inventory[i].getNumberOfItems());
    }
    System.out.printf("+-------------------------------------------------------------------------+\n");
    System.out.printf("| NUMBER OF ITEMS: %3d                                                    |\n",nreg);
    System.out.printf("+-------------------------------------------------------------------------+\n");
}

}

package DataBases;
import java.sql.*;


public class Mysql
{
// instance variables - replace the example below with your own
private String host;
private String user;
private String pass;
private String db;
private int port;
private Connection connection;

/**
 * Constructor for objects of class Mysql
 */
public Mysql()throws ClassNotFoundException,InstantiationException,IllegalAccessException
{
    // initialise instance variables
    this.host = "localhost";
    this.user = "inventory";
    this.pass = "123456";
    this.port = 3306;
    this.db = "inventory";
    this.Init();
}

public Mysql(String host,String user,String pass,int port,String db) throws ClassNotFoundException,InstantiationException,IllegalAccessException
{
    this.host = host;
    this.user = user;
    this.pass = pass;
    this.port = port;
    this.db = db;
    this.Init();
}


public void Init() throws ClassNotFoundException,InstantiationException,IllegalAccessException
{
    // put your code here
    Class.forName("com.mysql.jdbc.Driver").newInstance();
}

public void On() throws SQLException
{

    this.connection = DriverManager.getConnection("jdbc:mysql://"+ this.host +"/" + this.db,this.user,this.pass);
}

public void Off() throws SQLException
{

    this.connection.close();
}

public ResultSet ExeGet(String Query) throws SQLException
{
    Statement st = this.connection.createStatement();
    return (ResultSet) st.executeQuery(Query);
}

public int Exe(String Query) throws SQLException
{

    Statement st = this.connection.createStatement();
    return st.executeUpdate(Query);
}

}

look for Dao pattern: http://stackoverflow.com/questions/12812256/how-do-i-implement-a-dao-manager-using-jdbc-and-connection-pools — MGorgon, Feb 28 '16 at 12:25
Please learn the Java coding standards and follow them. Your method names should start with lower case letters. Your code would be harder for an experienced Java developer to read. Readability matters a great deal. Think about naming, too. Your names aren't good. — duffymo, Feb 28 '16 at 12:46

Benoit Vanalderweireldt · Answer 1 · 2016-02-28T15:10:55.423

To answer quickly yes it's a terrible design sorry, here is what I would do to improve it :

Try to achieve loose coupling between your classes, each class should be the implementation of an interface.For example.

public interface ConnectionManager {
  void On() throws SQLException;
  void Off() throws SQLException;
  ResultSet ExeGet(String Query) throws SQLException;
  int Exe(String Query) throws SQLException;
}

public class MysqlConnectionManager implements ConnectionManager {
  @Override
  public void On() throws SQLException {
  }
  @Override
  public void Off() throws SQLException {
  }
  @Override
  public ResultSet ExeGet(String Query) throws SQLException {
  }
  @Override
  public int Exe(String Query) throws SQLException {
  }
}

And then your DAOs would have a reference like this (this could be injected) :

    ConnectionManager connectionManager = new MysqlConnectionManager();

Try to follow a camel case practice (Wiki source)

Your attributes and associate methods should share the same names based on your attribute name.

private String barCode;

public String getBarCode(){
    return this.barCode;
}

public void setBarCode(final String barCode){
    this.barCode = barCode;
}

Follow the DAO pattern (Wiki source), basically you will need to create a class name ItemDao that would take care of all operation with the DB for 'Item'.
The Inventory should be renamed to ItemService, and it shouldn't have access to the DB only DAOs would, so if you need to implement another DAO it would easy to replace it.
Don't use System.out.print too much, instead configure your class to use an output stream, so it's easily configurable with a file or String output stream.
To inject data into your database use prepared statement instead of concatenating data inside your query (SQL injection)

Thanks for your answer. No problem i know my code is not very good hahaha, this about that improve. I've got an other question. Why is better implement interfaces?? every class I implement sould have an interface associated?? — ngry Sheep, Feb 28 '16 at 17:49
You are welcome, it's better because it makes your code loose coupled each class knows nothing about other classes implementation all they know is that the implementation class they are talking to respects the interface contract. So if you need for example to change you Connection to SQL Server the only theoretical change you will need to do is to add a new implementation of ConnectionManager and use it. — Benoit Vanalderweireldt, Feb 28 '16 at 19:50

Is this class design wrong?

1 Answers1