1
\$\begingroup\$

I have finished my first Android App, but I am getting lots of skipped frame messages from Choreographer. So, I started reading and got the impression that I should put as less calculations/works as possible in main thread, rather should use AsyncTask.

To understand the thumb rule, can anybody please validate my code whether I should move any methods in AsyncTask?

public class EditEntry extends Activity  implements DatePickerFragment.TheListener{
    public static int EDIT_BOOK_ID;
    EditText edit_datepurchased;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_edit_entry);

        Intent eintent = getIntent();
        EDIT_BOOK_ID = Integer.parseInt(eintent.getStringExtra("passid"));
        populateData(EDIT_BOOK_ID);
    }

    /**
     * Set up the {@link android.app.ActionBar}.
     */
    @TargetApi(Build.VERSION_CODES.HONEYCOMB)
    private void setupActionBar() {
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB) {
            getActionBar().setDisplayHomeAsUpEnabled(true);
        }
    }

    @Override
    public boolean onCreateOptionsMenu(Menu menu) {
        getMenuInflater().inflate(R.menu.edit_entry, menu);
        return true;
    }

    @Override
    public boolean onOptionsItemSelected(MenuItem item) {
        switch (item.getItemId()) {
        case R.id.editsave:
                editSave();
                return true;
        case android.R.id.home:
            String currlist = Integer.toString(EDIT_BOOK_ID);
                Intent editIntent = new Intent();
                editIntent.putExtra("bookid", currlist);
                setResult(RESULT_OK, editIntent);
                finish();
            return true;
        }
        return super.onOptionsItemSelected(item);
    }

    //populateData get the data from database and populate all the fields in the page

    public void populateData(int runid){
        mylibmandbhandler db = new mylibmandbhandler(this);
        mylibman cn = new mylibman();
        cn = db.getNextBook(runid,2);  // Method in mylibmandbhandler class to fetch data
        String temp;
        EditText mEditText = new EditText(this);

        mEditText = (EditText) findViewById(R.id.bookname); // TEXT
        mEditText.setText(cn.getBookname());

        mEditText = (EditText) findViewById(R.id.pages); //NUMBER
        temp = Integer.toString(cn.getPages());
        mEditText.setText(temp.equals("-1")?"":temp);

        CheckBox mCheckBox = (CheckBox) findViewById(R.id.inlibrary); //CHECKBOX
        if(cn.getInlibrary()==1){
            mCheckBox.setChecked(true);
        }else{
            mCheckBox.setChecked(false);
        }
        ....................
        ....................
        // I HAVE AROUND 25 fields (mostly EditText)
        ....................
        ....................

        db.close();
    }

    // editSave update the database based on changed values
    public void editSave() {
        AlertDialog.Builder editDialog = new AlertDialog.Builder(EditEntry.this);
        editDialog.setTitle("Confirm Edit...");
        editDialog.setMessage("Are you sure you want save changes?");
        final mylibmandbhandler db = new mylibmandbhandler(this);
        editDialog.setNegativeButton("NO", new DialogInterface.OnClickListener() {
            public void onClick(DialogInterface dialog, int which) {
                dialog.cancel();
            }
        });

        editDialog.setPositiveButton("YES", new DialogInterface.OnClickListener() {
            public void onClick(DialogInterface dialog,int which) {
                String[] dataarr = new String[17];
                EditText xEditText;
                String aTemp;
                int cnt = 0;
                xEditText = (EditText) findViewById(R.id.bookname);
                dataarr[cnt++] = xEditText.getText().toString();

                xEditText = (EditText) findViewById(R.id.pages);
                aTemp = xEditText.getText().toString();
                try {
                   Integer.parseInt(aTemp);  // IF NUMBER
                   dataarr[cnt++] = aTemp;
                } catch (NumberFormatException e) {
                    dataarr[cnt++] = "-1";
                }
                     .....................
                     .....................
                     // Get All 25 fields data
                     .....................
                     .....................


                    db.updateRecord(new mylibman(dataarr),EDIT_BOOK_ID);   // Method in mylibmandbhandler class to update database

                Toast.makeText(getApplicationContext(), "Changes saved for Book#"+EDIT_BOOK_ID, Toast.LENGTH_SHORT).show();

                //RETURN TO MAIN ACTIVITY PAGE AND RELOAD THE EDITED RECORD
                String currlist = Integer.toString(EDIT_BOOK_ID);
                Intent editIntent = new Intent();
                editIntent.putExtra("bookid", currlist);
                setResult(RESULT_OK, editIntent); 
                finish();
            }
        });
        db.close();
        editDialog.show();
    }

    //SHOW DATE DIALOG
    public void showdate(View v) {
        edit_datepurchased = (EditText) findViewById(v.getId());
        DialogFragment newFragment = new DatePickerFragment();
        Bundle bundle = new Bundle();
        bundle.putString("dateAsText",edit_datepurchased.getText().toString());
        newFragment.setArguments(bundle); 
        newFragment.show(getFragmentManager(), "datePicker");
    }

    //@Override
    public void returnDate(String date) {
        edit_datepurchased.setText(date);
    }

}
\$\endgroup\$
  • \$\begingroup\$ Possibly related: stackoverflow.com/questions/15963969/… . Are you running on an emulator or a real device? If you are running on a real device, does it respond quickly? If it does respond quickly I wouldn't worry too much. If it doesn't let me know and I can provide a more detailed answer. \$\endgroup\$ – Simon Forsberg Nov 23 '13 at 22:42
  • \$\begingroup\$ thanks, I didn't try on a real device so far .. I'll let you know after I try .. thanks again :) \$\endgroup\$ – abdfahim Nov 24 '13 at 3:04
2
\$\begingroup\$

Both the methods editSave() and populateData() require access to a database to function. They both should be modified to use AsyncTask in order to reduce the footprint that your code has on the 'main thread'.

I presume the database is local to the device because otherwise you would be getting the notorious NetworkOnMainThreadException... but regardless, for the same reason that NetworkOnMainThread has been designated as a critical problem, database access should be treated the same way. Any time you have IO or other non-CPU bound activities, you really should be using the AsyncTask, and there's a good example on StackOverflow too.

\$\endgroup\$
  • \$\begingroup\$ Because populateData() is not a class ( or at least it doesn't seem to be to me), would AsyncTask need to be added to getNextBook? \$\endgroup\$ – Chillie Mar 18 '14 at 20:06

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.