3
\$\begingroup\$

I am developing an application to validate the interaction between activities through intents.

I created 5 ImageButtons with an image for each. Each button is a movie and if you click any of them, it's directed to a new activity with a synopsis of the film. In the activity with the synopsis, there is an "up navigation" that returns to the MainActivity (home).

The way I developed left a very extensive project since I created six activities. (one main activity and 5 synopsis activities, one for each film) as well as 6 layouts. Also, my APK is to 1.5 MB in size.

Could someone help me with suggestions for best practices on how to minimize my code or how developed is correct and could be developed in a real application?

My MainActivity

package luizugliano.com.br.appfilmes;
import android.content.Context;
import android.content.Intent;
import android.os.Bundle;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.Toolbar;
import android.view.Menu;
import android.view.MenuItem;
import android.view.View;
public class MainActivity extends AppCompatActivity {
@Override
protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 setContentView(R.layout.activity_main);
 Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
 setSupportActionBar(toolbar);
}
public void onClickBtVideo01(View view){
 Intent intent = new Intent(getContext(),ActivityVideo01.class);
 startActivity(intent);
}
public void onClickBtVideo02(View view){
 Intent intent = new Intent(getContext(),ActivityVideo02.class);
 startActivity(intent);
}
public void onClickBtVideo03(View view){
 Intent intent = new Intent(getContext(),ActivityVideo03.class);
 startActivity(intent);
}
public void onClickBtVideo04(View view){
 Intent intent = new Intent(getContext(),ActivityVideo04.class);
 startActivity(intent);
}
public void onClickBtVideo05(View view){
 Intent intent = new Intent(getContext(),ActivityVideo05.class);
 startActivity(intent);
}
private Context getContext(){
 return this;
}
@Override
public boolean onCreateOptionsMenu(Menu menu) {
 // Inflate the menu; this adds items to the action bar if it is present.
 getMenuInflater().inflate(R.menu.menu_main, menu);
 return true;
}
@Override
public boolean onOptionsItemSelected(MenuItem item) {
 // Handle action bar item clicks here. The action bar will
 // automatically handle clicks on the Home/Up button, so long
 // as you specify a parent activity in AndroidManifest.xml.
 int id = item.getItemId();
 //noinspection SimplifiableIfStatement
 if (id == R.id.action_settings) {
 return true;
 }
 return super.onOptionsItemSelected(item);
}
}

My ActivityVideo01 (other activities have the same code so I only put this as an example)

package luizugliano.com.br.appfilmes;
import android.os.Bundle;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.Toolbar;
import android.view.MenuItem;
public class ActivityVideo01 extends AppCompatActivity {
@Override
protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 setContentView(R.layout.activity_activity_video01);
 Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
 setSupportActionBar(toolbar);
 getSupportActionBar().setDisplayHomeAsUpEnabled(true);
}
@Override
public boolean onOptionsItemSelected(MenuItem item) {
 // Handle action bar item clicks here. The action bar will
 // automatically handle clicks on the Home/Up button, so long
 // as you specify a parent activity in AndroidManifest.xml.
 int id = item.getItemId();
 //noinspection SimplifiableIfStatement
 if (id == android.R.id.home) {
 //O método finish encerrará essa activity
 finish();
 return true;
 }
 return super.onOptionsItemSelected(item);
}
}

My content_main.xml

<?xml version="1.0" encoding="utf-8"?>
<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
xmlns:app="http://schemas.android.com/apk/res-auto" android:layout_width="match_parent"
android:layout_height="match_parent" android:paddingLeft="@dimen/activity_horizontal_margin"
android:paddingRight="@dimen/activity_horizontal_margin"
android:paddingTop="@dimen/activity_vertical_margin"
android:paddingBottom="@dimen/activity_vertical_margin"
app:layout_behavior="@string/appbar_scrolling_view_behavior"
tools:showIn="@layout/activity_main" tools:context=".MainActivity">
<TextView android:text="Sinopse - Filmes" android:layout_width="wrap_content"
 android:layout_height="wrap_content"
 android:layout_alignParentTop="true"
 android:layout_centerHorizontal="true"
 android:textSize="22dp"/>
<HorizontalScrollView
 android:layout_width="wrap_content"
 android:layout_height="wrap_content">
<LinearLayout
 android:orientation="horizontal"
 android:layout_width="match_parent"
 android:layout_height="match_parent"
 android:layout_centerHorizontal="true"
 android:layout_marginTop="@dimen/layout_marginTop">
 <ImageButton
 android:layout_width="@dimen/layout_width"
 android:layout_height="@dimen/layout_height"
 android:id="@+id/imageButton01"
 android:layout_centerVertical="true"
 android:layout_centerHorizontal="true"
 android:layout_gravity="center_vertical"
 android:background="@drawable/btn_img_01"
 android:onClick="onClickBtVideo01"/>
 <ImageButton
 android:layout_width="@dimen/layout_width"
 android:layout_height="@dimen/layout_height"
 android:id="@+id/imageButton02"
 android:layout_centerVertical="true"
 android:layout_centerHorizontal="true"
 android:layout_gravity="center_vertical"
 android:layout_marginLeft="@dimen/layout_marginLeft"
 android:background="@drawable/btn_img_02"
 android:onClick="onClickBtVideo02"/>
 <ImageButton
 android:layout_width="@dimen/layout_width"
 android:layout_height="@dimen/layout_height"
 android:id="@+id/imageButton03"
 android:layout_centerVertical="true"
 android:layout_centerHorizontal="true"
 android:layout_gravity="center_vertical"
 android:layout_marginLeft="@dimen/layout_marginLeft"
 android:background="@drawable/btn_img_03"
 android:onClick="onClickBtVideo03"/>
 <ImageButton
 android:layout_width="@dimen/layout_width"
 android:layout_height="@dimen/layout_height"
 android:id="@+id/imageButton04"
 android:layout_centerVertical="true"
 android:layout_centerHorizontal="true"
 android:layout_gravity="center_vertical"
 android:layout_marginLeft="@dimen/layout_marginLeft"
 android:background="@drawable/btn_img_04"
 android:onClick="onClickBtVideo04"/>
 <ImageButton
 android:layout_width="@dimen/layout_width"
 android:layout_height="@dimen/layout_height"
 android:id="@+id/imageButton05"
 android:layout_centerVertical="true"
 android:layout_centerHorizontal="true"
 android:layout_gravity="center_vertical"
 android:layout_marginLeft="@dimen/layout_marginLeft"
 android:background="@drawable/btn_img_05"
 android:onClick="onClickBtVideo05"/>
</LinearLayout>
</HorizontalScrollView>
</RelativeLayout>

My content_activity_video01.xml (other layout have the same code so I only put this as an example)

<?xml version="1.0" encoding="utf-8"?>
<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
xmlns:app="http://schemas.android.com/apk/res-auto" android:layout_width="match_parent"
android:layout_height="match_parent" android:paddingLeft="@dimen/activity_horizontal_margin"
android:paddingRight="@dimen/activity_horizontal_margin"
android:paddingTop="@dimen/activity_vertical_margin"
android:paddingBottom="@dimen/activity_vertical_margin"
app:layout_behavior="@string/appbar_scrolling_view_behavior"
tools:showIn="@layout/activity_activity_video01"
tools:context="luizugliano.com.br.appfilmes.ActivityVideo01">
<TextView
 android:layout_width="wrap_content"
 android:layout_height="wrap_content"
 android:textAppearance="?android:attr/textAppearanceMedium"
 android:text="Title Synopsis"
 android:id="@+id/textView"
 android:layout_alignParentTop="true"
 android:layout_centerHorizontal="true" />
<TextView
 android:layout_width="wrap_content"
 android:layout_height="wrap_content"
 android:textAppearance="?android:attr/textAppearanceMedium"
 android:text="Synopsis"
 android:id="@+id/textView2"
 android:layout_below="@+id/textView"
 android:layout_marginTop="51dp"
 android:layout_alignParentRight="true"
 android:layout_alignParentEnd="true" />
</RelativeLayout>
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 20, 2015 at 13:15
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. For examples of good titles, check out Best of Code Review 2014 - Best Question Title Category You may also want to read How to get the best value out of Code Review - Asking Questions. \$\endgroup\$ Commented Oct 20, 2015 at 13:23
  • 1
    \$\begingroup\$ I don't have enough experience with Android to make a real answer, but you should definitely try to have just one activity and one layout for your movie view. You should then programmatically set the synopsis and other layout options specific to that movie. If you're ever copy pasting a lot of code then there's usually a way to repeat it with the small changes you need. \$\endgroup\$ Commented Oct 20, 2015 at 13:27

1 Answer 1

3
\$\begingroup\$

From the actual organization of your application, here's few things to minimize your code:

1. Single classes

As @superbiasedman pointed out, as far as you will show the same thing, use a single layout and a single class. For example, let's say you have two activities MainActivity and ActivityVideo, the last one will always be the same for all video movie and you will avoid too many unnecessary lines of codes.

You can pass different datas to the same activity which can change its behaviour according to it received. Like this, from the first activity:

Intent intent = new Intent(this, ActivityVideo.class); // call the same activity
intent.putInt("MovieId", 4052);
intent.putString("MovieName", "This is a movie"); // send the details
...
startActivity(intent); 

to the second one:

Bundle extras = getIntent().getExtras(); // retrieve passed datas
int movieId = extras.getInt("MovieId", 0); // 2nd param is the default one
String name = extras.getString("MovieName", ""); 
String synopsis = extras.getString("MovieSynopsis", ""); 
...

And you'll change the detail for each selected items (like the title and the synopsis) with only one layout content_activity_video.xml like this:

textView.setText(name);
textView2.setText(synopsis);

Therefore instead of 5 classes and 5 layouts, you'll have just 1 for each.

2. Minimize duplicates methods:

Call just one method to show the same ActivityVideo (I assume following step1). Using a switch and global variables is useful to do the same job with specific changes:

public void onMovieClick(View view) {
 // prepare your intent with detail movie class
 Intent intent = new Intent(this, ActivityVideo.class);
 // attach the datas according to each movie 
 // (thanks to getId() methods to get movie view clicked)
 switch (view.getId()) {
 // movie one
 case R.id.imageButton01:
 intent.putInt("MovieId", 4052);
 intent.putString("MovieName", "This is a movie");
 ...
 break;
 // movie two
 case R.id.imageButton02:
 intent.putInt("MovieId", 8611);
 intent.putString("MovieName", "Another movie");
 ...
 break;
 }
 // call detail movie class
 startActivity(intent);
}

Then the ImageButtons will call this method, instead of as many methods as many buttons:

android:onClick="onMovieClick"

3. Think of layout's redrawing:

Avoid, as far as you can, RelativeLayout. It calls a redraw for all attributes regarding another view (like toLeftOf, centerHorizontal..). Instead, when you have a simple layout, try LinearLayout which draws its child views just one time. Your main_activity.xml could be without unnecessary redraw:

<!-- use a LinearLayout with orientation to vertical: all 
 the children will be displayed one below the others -->
<LinearLayout
 orientation="vertical" ...>
 <!-- set the content to fill width screen (width=match_parent)
 and display the text at the center of it (gravity=center) -->
 <TextView
 android:layout_width="match_parent"
 android:layout_height="wrap_content"
 android:gravity="center" ... />
 <...other views...>
</...>

4. And what's next?

Maybe with the 3 steps above, it looks close to a 'real' application. However, all depends on what you need and what you want in your app.

If I assumed your application is on the Google Play and show news movies, a 'real' app should have a dynamic pattern, a dynamic structure. Therefore, you will need to inform about GridView works (see how to get a custom horizontal one according to the number of movies displayed) and adapter: to use one layout for each movie's item, recycling dynamically the item views and displaying into a horizontal list.
That can be the next step, you can find many tutorial on this (and libraries to get it on horizontal).
But for now, I think it's a good start to optimize your code.

Mast
13.8k12 gold badges57 silver badges127 bronze badges
answered Nov 1, 2015 at 6:02
\$\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.