4
\$\begingroup\$

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<Map<String, String>> itemData = new ArrayList<>();
private Map<String, String> 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.

\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

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<String, String> 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++;
    }
    
\$\endgroup\$
3
  • \$\begingroup\$ thanks a lot can you help improve my ArrayList implementation as well.Thanks \$\endgroup\$ Nov 30, 2015 at 2:12
  • \$\begingroup\$ @AndroidNewBee What ArrayList implementation? \$\endgroup\$ Nov 30, 2015 at 2:29
  • \$\begingroup\$ instead of putting the data into an ArrayList and then making it a jsonArray. Can't I just put all the data directly into an jsonArray. Would that improve my app performance? \$\endgroup\$ Nov 30, 2015 at 2:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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