Creating an ArrayList of Maps

I am making an ArrayList that contains Map values. itemData is my ArrayList that contains itemselected, which is a map. Then, because I want to send this array to my PHP code, I am converting itemData into a JSONArray. Then parsing the array in my PHP script and inserting those individual values into a MySQL database.

private ArrayList> itemData = new ArrayList<>();
private Map itemselected = new HashMap<>();
JSONArray itemSelectedJson = new JSONArray();

 private void selectedItems() {
        if(invEstSwitch.isChecked())
        {
            billType = textViewEstimate.getText().toString();
        }else{
            billType = textViewInvoice.getText().toString();
        }

        itemselected.put("custInfo",custSelected.toString());
        itemselected.put("invoiceNo", textViewInvNo.getText().toString());
        itemselected.put("barcode", barCode.getText().toString());
        itemselected.put("desc", itemDesc.getText().toString());
        itemselected.put("weight", weightLine.getText().toString());
        itemselected.put("rate", rateAmount.getText().toString());
        itemselected.put("makingAmt", makingAmount.getText().toString());
        itemselected.put("net_rate", netRate.getText().toString());
        itemselected.put("itemTotal", itemtotal.getText().toString());
        itemselected.put("vat", textViewVat.getText().toString());
        itemselected.put("sum_total", textViewSum.getText().toString());
        itemselected.put("bill_type", billType);
        itemselected.put("date", textViewCurrentDate.getText().toString());

        //Add the map to the Array
        itemData.add(index, itemselected);
        itemSelectedJson= new JSONArray(Arrays.asList(itemData));
        index++;
    }

The code is pretty straightforward, but I am interested in any possible improvement. I am sure there is a better way to do it.

Just some convention points:

• The indentation is off in the first couple of lines. Is this due to copy-paste? I have to assume so, as the rest of your code seems fine.

-
The following if statement:

if(invEstSwitch.isChecked())
{
    billType = textViewEstimate.getText().toString();
}else{
    billType = textViewInvoice.getText().toString();
}

is inconsistent. You put a newline before the beginning brace at the if, and not at the else.

It is also bad formatting. There should be a space both left and right of the else, and a space after if:

if (invEstSwitch.isChecked()) {
    billType = textViewEstimate.getText().toString();
} else {
    billType = textViewInvoice.getText().toString();
}

-
The if statement can be simplified into a ternary expression, as it is assigning the same thing (assuming textViewEstimate and textViewInvoice are the same type):

billType = (invEstSwitch.isChecked() ? textViewEstimate : textViewInvoice)
        .getText().toString();

-
Try not to use ArrayList. As I said in this answer:


It is almost always a poor option, as it is pretty much an array with
helper functions. If you have an ArrayList of 10000 integers, and
then add another integer, ArrayList will (if its current array it's
storing the values in is of size 10000) create a new array, move all
the values of the old array in the new array, then add the integer to
the end. Sounds inefficient? Certainly does.


My opinion is to use LinkedList. LinkedList is faster, because it
works like this:

• Each value in the list is stored in a Node.

• Each Node points to the next Node.

• Adding something to the end is as simple as creating a new Node and linking it to the Node chain.

• Removing and inserting is as simple as changing some links around. The problem would be to find that link.

Worst-case time complexity comparison:

LinkedList        ArrayList
Get:       O(n)              O(1)
Add:       O(n)              O(n)
Insert:    O(n)              O(n)

Best-case time complexity comparison:

LinkedList        ArrayList
Get:       O(1)              O(1)
Add:       O(1)              O(1)
Insert:    O(1)              O(n) <- assuming not inserting at end of list

Yes, LinkedList is losing, but in this case, you don't use get
often. On the other hand, memory is an issue for ArrayList:

LinkedList        ArrayList
Get:       O(1)              O(1)
Add:       O(1)              O(n)
Insert:    O(1)              O(n)

-
Your method is fairly inefficient. You create a new JSONArray each time you call the method. Instead, use the given put() method (this also means you don't need an ArrayList):

private Map itemselected = new HashMap<>();
JSONArray itemSelectedJson = new JSONArray();

private void selectedItems() {
    billType = (invEstSwitch.isChecked() ? textViewEstimate : textViewInvoice)
            .getText().toString();

    itemselected.put("custInfo",custSelected.toString());
    itemselected.put("invoiceNo", textViewInvNo.getText().toString());
    itemselected.put("barcode", barCode.getText().toString());
    itemselected.put("desc", itemDesc.getText().toString());
    itemselected.put("weight", weightLine.getText().toString());
    itemselected.put("rate", rateAmount.getText().toString());
    itemselected.put("makingAmt", makingAmount.getText().toString());
    itemselected.put("net_rate", netRate.getText().toString());
    itemselected.put("itemTotal", itemtotal.getText().toString());
    itemselected.put("vat", textViewVat.getText().toString());
    itemselected.put("sum_total", textViewSum.getText().toString());
    itemselected.put("bill_type", billType);
    itemselected.put("date", textViewCurrentDate.getText().toString());

    // Add the map to the Array
    itemSelectedJson.put(itemselected);
    index++;
}

StackExchange Code Review Q#112201, answer score: 3