4
\$\begingroup\$

I have been slowly getting more and more "at home" with Android development and I would like some guidance. I have made an app that uses a ListActivity, that attaches to an SQLite DB. I'm using the onListItemClick function to fire up a "single record view" where I can delete and edit the record. There is also a way to edit the record from the list view by using the Context Menu and selecting so there are potentially 2 ways to edit a record.

  • Directly from a ListView via Context Menu
  • Selecting Edit from a single record view

Both the single record view and the edit view are being handled by the same Activity (not sure if this is common practice or not). Now having been a programmer for some time something tells me that I am not doing this in the best way that I could so I thought I would submit it for code review. So here is the "overly-complicated feeling" Activity which I have commented to try and help understand my logic. I did think that this might be something I should be using a FrameLayout for but don't really know if that is the best way to do things. I am still new to Android dev so have a lot to learn.

public class CardActivity extends Activity implements OnClickListener {
 private Card card;
 private EditText questionEdit;
 private EditText answerEdit;
 private CardSQLiteHelper dbHelper;
 @Override
 protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 dbHelper = new CardSQLiteHelper(this);
 dbHelper.open();
 Bundle b = getIntent().getExtras();
 if(b == null) { // Coming in from Add there is no card in the Intent
 card = new Card();
 setupEdit();
 } else if( b.containsKey(Intent.ACTION_VIEW)) { // Coming in from View the card is stored using the ACTION_VIEW key
 card = (Card) b.get(Intent.ACTION_VIEW);
 setupView();
 } else { // Coming from Edit the card is stored using the ACTION_EDIT key
 card = (Card) b.get(Intent.ACTION_EDIT);
 setupEdit();
 }
 }
 /**
 * Sets up the view for editing using the right layout and registering the buttons
 */
 private void setupEdit() {
 setContentView(R.layout.card_edit);
 questionEdit = (EditText) findViewById(R.id.questionEditText);
 answerEdit = (EditText) findViewById(R.id.answerEditText);
 questionEdit.setText(card.getQuestion());
 answerEdit.setText(card.getAnswer());
 Button saveButton = (Button) findViewById(R.id.cardSaveButton);
 Button cancelButton = (Button) findViewById(R.id.cardCancelButton);
 saveButton.setOnClickListener(this);
 cancelButton.setOnClickListener(this);
 }
 /**
 * Sets up the view for "Single record View" mode displaying the data and offering a Delete and Edit button
 */
 private void setupView() {
 setContentView(R.layout.card_view);
 TextView questionText = (TextView) findViewById(R.id.questionTextView);
 TextView answerText = (TextView) findViewById(R.id.answerTextView);
 questionText.setText(card.getQuestion());
 answerText.setText(card.getAnswer());
 Button deleteButton = (Button) findViewById(R.id.cardDeleteButton);
 Button editButton = (Button) findViewById(R.id.cardEditButton);
 deleteButton.setOnClickListener(this);
 editButton.setOnClickListener(this);
 }
 @Override
 public void onClick(View v) {
 switch (v.getId()) {
 case R.id.cardEditButton: 
 // Clicked the Edit button in the "Single record view" so we switch to an Edit view
 setupEdit();
 break;
 case R.id.cardDeleteButton:
 // Delete is easy, just nuke the card and return an OK result
 dbHelper.deleteCard(card);
 setResult(RESULT_OK);
 finish();
 break;
 case R.id.cardCancelButton:
 // Cancel again is easy, do nothing and return the CANCELLED result
 setResult(RESULT_CANCELED);
 finish();
 break;
 case R.id.cardSaveButton:
 // Save is not hard just a bit lengthy because of the things we need to do
 // Check there is content in the Question EditText
 if(questionEdit.getText().toString().length() == 0) {
 Toast.makeText(this, "Your question must have some text", Toast.LENGTH_LONG).show();
 break;
 }
 // Check there is content in the Answer EditText
 if(answerEdit.getText().toString().length() == 0) {
 Toast.makeText(this, "Your answer must have some text", Toast.LENGTH_LONG).show();
 break;
 }
 // Put the text into my object
 card.setQuestion(questionEdit.getText().toString());
 card.setAnswer(answerEdit.getText().toString());
 // If the card has no ID then we are adding a new card
 if(card.getId() == 0) {
 dbHelper.addCard(card);
 } else {
 dbHelper.updateCard(card);
 }
 setResult(RESULT_OK);
 finish();
 break;
 }
 }
}

As requested this is a bit more of an explanation of what the app does. It's a basic app that I am using as a learning process, of a revision or learning card system. The end user can add & remove cards from a list which they can store a Question and Answer on as if you were using them to revise for an exam, eventually I will give the user a way to randomise the cards and answer the relevant question as well as provide more ways to store data and get between the cards.

asked Jan 11, 2014 at 2:28
\$\endgroup\$
2
  • \$\begingroup\$ Do you think you could provide a little more context for what the app does? I think I understand most of what's going on, but it helps to have an explanation behind it. \$\endgroup\$ Commented Jan 11, 2014 at 5:24
  • \$\begingroup\$ Certainly I will edit the main text \$\endgroup\$ Commented Jan 11, 2014 at 7:10

1 Answer 1

1
\$\begingroup\$

One of the tenets fo SOLID programming (the first one, in fact) is that classes should have a single responsibility.

Here you have a class which has two distinct functions: view a card ; edit a card.

The code complexity you have introduced just to differentiate between the uses of your class is more than the complexity of what you would have if you had two classes.

Your instinct is right, you are not doing this the right way.

In this particular case, I would recommend copy/pasting the class, and making each version a specialization for either View or Edit. The setupEdit and setupView should instead be incorporated in to the onCreate() method. The onClick() logic can be split quite simply as well.

What is nice about this is that the code becomes much more reusable... you can have the two specialized classes, and adding a new way to access the Activity (swiping '>next', etc.) would be much easier to do.

answered Feb 3, 2014 at 0:40
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.