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>
\$\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\$ 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\$ 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.

\$\endgroup\$

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.