1
\$\begingroup\$

I am learning MVC and Android. I have create a sample program which is supposed to implement the MVC philosophy. I plan to build my codes on top of this.

I wish to know whether I have got the concept more or less, or if there are any fundamental issues in my understanding of the MVC.

Summary: This application will get a city name and display it with a few concatenations (country name etc).

Model -> Model.java

/**
 * 
 */
package abc.samples.mvc;
/**
 * @author Rahul
 *
 */
public class Model {

    String mData;
    String mInput;
    String mOutput;

    public void set(String temp) {
        mInput = temp;
        mData = mInput + ", India.";
    }

    public String get() {
        mOutput = "You are from "+mData;
        return mOutput;
    }

}

View -> view.xml

<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    tools:context="${relativePackage}.${activityClass}" >

    <Button
        android:id="@+id/vControl"
        android:layout_width="fill_parent"
        android:layout_height="wrap_content"
        android:layout_alignParentLeft="true"
        android:layout_alignParentTop="true"
        android:text="Control" />

    <EditText
        android:id="@+id/vInput"
        android:layout_width="fill_parent"
        android:layout_height="wrap_content"
        android:layout_alignParentLeft="true"
        android:layout_below="@+id/vControl"
        android:ems="10" >
        <requestFocus />
    </EditText>

    <TextView
        android:id="@+id/vOutput"
        android:layout_width="fill_parent"
        android:layout_height="wrap_content"
        android:layout_alignParentLeft="true"
        android:layout_below="@+id/vInput"
        android:text="Large Text"
        android:textAppearance="?android:attr/textAppearanceLarge" />
</RelativeLayout>

Controller -> Controller.java

package abc.samples.mvc;

import android.app.Activity;

import android.os.Bundle;
import android.view.Menu;
import android.view.MenuItem;
import android.view.View;
import android.widget.Button;
import android.widget.EditText;
import android.widget.TextView;

public class Controller extends Activity {

    EditText cInput;
    TextView cOutput;
    Button cControl;
    Model cData;

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

        cInput = (EditText) findViewById(R.id.vInput);
        cOutput = (TextView) findViewById(R.id.vOutput);
        cControl = (Button) findViewById(R.id.vControl);

        cControl.setOnClickListener(new View.OnClickListener() {

            @Override
            public void onClick(View v) {

                cData = new Model();

                cData.set(cInput.getText().toString());
                cOutput.setText(cData.get());
            }
        });
    }
}
\$\endgroup\$
2
\$\begingroup\$

A model should not have presentation logic, and be mostly about containing data. The text "You are from ..." Doesn't belong in a model. As it is, your model includes logic that concerns representation, and so you're violating MVC. You could use another class to be in charge of formatting a Model instance. In other words, it would be best if your Model just contained fields and did nothing else. Furthermore, all fields can be private final, for example:

public class City {

    private final String name;

    public City(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }
}

I removed the other fields which seemed quite pointless for a model. And I renamed Model to City, because "Model" is way too abstract and not useful.

In the activity it's strange to create fields when you only use them in onCreate. It's best to limit variables to the smallest scope necessary. As such, all these fields should be local variables in onCreate, like this:

EditText input = (EditText) findViewById(R.id.vInput);
TextView output = (TextView) findViewById(R.id.vOutput);
Button control = (Button) findViewById(R.id.vControl);

These names are terrible by the way. (Now I realize your question may have been rather hypothetical, not a real program.) Give them better names.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks. I understood. 1. Models should not mess with the formatting of data. (It can transform values (mph to kmpph etc)). 2. My model should have only contained String mData. The get and set function should have been defined in another class (Say Transform) and my Controller should call a transformer method with data, which in turn will transform the data and send it the data to model. Thanks a lot. Hope I am right this time. Am just brainstorming about these, so your comments sure did help. \$\endgroup\$ – Rahul Nov 21 '14 at 10:51
1
\$\begingroup\$

I'd say you've more or less gotten the right idea. What i would like to change is in your onClick method you initialize a new Model. Move this initialization outisde of onClick.(You can still call for model inside onClick by writing:

Controller.model.this.set("string");

Another thing i react to is all instance variables have default visibility. You should have every single one private, both in the controller and in the model.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks. I will take care that all fields should be private. But one doubt regarding initialization outside of onClick. My logic was that memory is not used up until the button is clicked (Assume that I also freed the memory at the end of onclick). But now I think, though that logic is fine, the issue is that onclick should only focus on core thing that should happen when a button is clicked and NOT the auxiliary stuff. So, what you said seems to be more valid in general. \$\endgroup\$ – Rahul Nov 21 '14 at 10:56

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.