1
\$\begingroup\$

I've following code that insert single row in if (rows == 1) { part and inside loop it is batch insert.
So I'm thinking to skip single insert and have it in batch insert code.

private void jButton1ActionPerformed(java.awt.event.ActionEvent evt) {

        int rows = jTable.getRowCount();
        if (rows == 1) {
            String itemName = (String) jTable.getValueAt(0, 0);
            int itemQty = (int) jTable.getValueAt(0, 1);
            Double itemPrice = (Double) jTable.getValueAt(0, 2);

            Items items = new Items();
            items.setName(itemName);
            items.setPrice(itemPrice);
            items.setQty(itemQty);
            items.setTransactionNumber(manager.getTransNo());
            manager.saveItems(items);

Above part insert single record if there's only one row in a JTable Below in else part inside loop performed batch insert if JTable contains more than one row.

        } else {
            for (int i = 0; i < 1; i++) {
                String itemName = (String) jTable.getValueAt(i, 0);
                int itemQty = (int) jTable.getValueAt(i, 1);
                Double itemPrice = (Double) jTable.getValueAt(i, 2);
                Items items = new Items();
                items.setName(itemName);
                items.setPrice(itemPrice);
                items.setQty(itemQty);
                items.setTransactionNumber(manager.getTransNo());
                int max = items.getTransactionNumber();
                manager.saveItems(items);

And here in a second loop it does perform insertion after first row in a JTable when int i = 1.

for (i = 1; i < rows; i++) {
            String itemsName = (String) 
            jTable.getValueAt(i, 0);
            int itemsQty = (int) jTable.getValueAt(i, 1);
            Double itemsPrice = (Double) 
            jTable.getValueAt(i, 2);

            items.setName(itemsName);
            items.setPrice(itemsPrice);
            items.setQty(itemsQty);
            items.setTransactionNumber(max);
            manager.saveItems(items);
            }
        }
    }
}

I was thinking to save all rows without considering is either only one row or more.
So my modified version would not have if (rows == 1) {....} part
Will have only for (int i = 0; i <= 0; i++) {
So if there will have only one row in a JTable would this modified version can be cause of lower performance?

\$\endgroup\$
  • 1
    \$\begingroup\$ No, absolutely no MCVEs on Code Review. We need to see your real code and a description of what it does (and why). Otherwise we can't say much useful things about it. It's all explained in the link provided. In this case, a usage example could be a useful part of the description. \$\endgroup\$ – Mast Jan 30 at 9:28
1
\$\begingroup\$

Short answer to your question: No it's the same performance wise.


Full review:

My first concern with your code is the double for loop that uses the same looping variable i. This makes it harder to understand when you see an i inside the inner for loop.

My other major concern is that the only difference between the code before the inner for loop and the code inside the inner loop is setting the variable max which I had no idea what it should be at first sight. Let's start by renaming that to transactionNumber. And since this is independent of the loop itself let's initialise that from the manager right above the loops instead:

int transactionNumber = manager.getTransNo();
for (int i = 0; i < 1; i++) {
    String itemName = (String) jTable.getValueAt(i, 0);
    int itemQty = (int) jTable.getValueAt(i, 1);
    Double itemPrice = (Double) jTable.getValueAt(i, 2);
    Items items = new Items();
    items.setName(itemName);
    items.setPrice(itemPrice);
    items.setQty(itemQty);
    items.setTransactionNumber(transactionNumber);
    manager.saveItems(items);

    for (i = 1; i < rows; i++) {
        String itemsName = (String) jTable.getValueAt(i, 0);
        int itemsQty = (int) jTable.getValueAt(i, 1);
        Double itemsPrice = (Double) jTable.getValueAt(i, 2);

        items.setName(itemsName);
        items.setPrice(itemsPrice);
        items.setQty(itemsQty);
        items.setTransactionNumber(transactionNumber);
        manager.saveItems(items);
    }
}

Now with a closer look there's a second difference. You only instantiate items once and overwrite that with the setters. I don't think this is a good idea. You should probably create a new items object each time with it's own name, price, quantity and save that independent of any previous values. This means we can just combine the 2 for loops into 1 like so:

int transactionNumber = manager.getTransNo();
for (int i = 0; i < rows; i++) {
    String itemName = (String) jTable.getValueAt(i, 0);
    int itemQty = (int) jTable.getValueAt(i, 1);
    Double itemPrice = (Double) jTable.getValueAt(i, 2);
    Items items = new Items();
    items.setName(itemName);
    items.setPrice(itemPrice);
    items.setQty(itemQty);
    items.setTransactionNumber(transactionNumber);
    manager.saveItems(items);
}

If you now take a close look at what would happen if you only had 1 row you can see that the code inside the loop gets executed exactly 1 time with the i replaced by a 0. This is exactly the same code you have inside your if(rows == 1) block so you can remove that check and only keep the for loop.

Depending on how the Items are used in the rest of your code I also strongly suggest to remove the setters and pass any needed parameters with the constructor instead. This results in the following implementation of the entire method:

private void jButton1ActionPerformed (java.awt.event.ActionEvent evt){
    int transactionNumber = manager.getTransNo();
    for (int i = 0; i < rows; i++) {
        String itemName = (String) jTable.getValueAt(i, 0);
        int itemQty = (int) jTable.getValueAt(i, 1);
        Double itemPrice = (Double) jTable.getValueAt(i, 2);
        Items items = new Items(transactionNumber, itemName, itemPrice, itemQty);
        manager.saveItems(items);
    }
}

The only improvement you should consider is to rename jTable to something more meaningful.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for your efforts. I was thinking it's big deal with either original code or modified code. I think there's no need of second loop if I would remove check I had cause of rest of table rows. I was thinking If loop is burden for single row. Thanks a lot. \$\endgroup\$ – Swapnil Nagtilak Jan 30 at 10:50

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.