Android: determining if code should reside in activity or custom view [closed]

Question

As I've been getting familiar with working with Activities and custom Views, I've been constantly having to decide where code that is relevant to both a View and it's parent Activity should go. Add in custom objects that need to be accessed by both, and the options on how to structure the code are endless. Here's the specifics of my issue:

Relevant Classes/Files:
GameActivity extends Activity: the layout it uses contains some custom Views.

MapView extends View: this is contained in the layout used by GameActivity

World: a custom class that defines a custom World object

Expected Result:
GameActivity pulls up the layout with MapView in it. The onDraw() function of the MapView draws a map on the canvas using information from a World object.

Problem:
The World object that the MapView needs is loaded from a previously saved file. I can get that object to the MapView in a number of different ways, and that's where I'm getting indecisive. I've gone through the following iterations. (Note, they all work. What I'm looking for is reasons to use one way over another.)

Version1:
GameActivity: all it does is setContentLayout(layout_with_mapview_in_it.xml)

MapView: has all the code for loading the World object from a file, and uses it to reference needed drawing parameters

World: a simple custom class w/ 3 parameters in its constructor

Then I decided that loading the World object from a file should be a method in the World class itself, instead of something that is done in the onCreate() method of the MapView. Since loading a world file is something you will never do without a World object, it makes sense that it should be class method. So I created a loadWorld(String world_name) method in the World class. Now the files looked like this:

Version2:
GameActivity: all it does is setContentLayout(layout_with_mapview_in_it.xml)

MapView: Creates a new World object using constructor, then calls it's loadWorld() method to update it with file info

World: a simple custom class w/ 3 parameters in it's constructor, and a loadWorld() method

Finally, I decided that only drawing activities should be in the MapView. The whole point of having a World object is to be able to pass it around, right? So I moved the World construction/loading out of the View and into the Activity. This resulted in having to create a setter method in the MapView to be able to pass the World object from the parent activity where it's getting created.

Version3:
GameActivity: sets layout, creates a World object and calls it's loadWorld() method to load it from the file. References the MapView by Id, then calls the setWorld() method of the MapView to pass the instance of the World object over

MapView: World object is set from outside. Draws the map using info from this object

World: a simple custom class w/ 3 parameters in it's constructor, and a loadWorld() method.

Ok, so that's where I'm currently at. My issue is that while I like the conventions I have picked in terms of having Views only contain drawing related code, and keeping class-related methods in their own classes- it seems when I switch to that method I'm suddenly creating temporary objects more often and passing objects from activity to activity to view and such. This seems like a lot more overhead, but at the same time that's the whole point of abstraction right? Abstract out a class so you can then instantiate an object from it and pass that around. Yet somehow it seems to me that the more I abstract out things, the more complex handling objects become.

I guess what I'm trying to ask here is, am I making things a ton more complicated by just not loading the World from the file in the MapView itself? Am I just being stubborn in not wanting to have code that involves reading object from a file in a View class? Is it any better to have it in its own class or in an Activity? What do I need to take into consideration when making these decisions? Is there a solution to my dilemma that I'm not even aware of?

I think the answer is going to be personal preference, but I want to get an idea if there are conventions out there to do it one way or the other, or if there are solid reasons why to structure a certain way. I've been trying to search for answers to this question, but I think I'm using the wrong search terms. I keep running across stuff that describes how to structure the activity/view structure, but nothing about the code inside. When there is code, it's inevitably someone teaching how to pass data between activities, or between an activity and view, etc. I know all those methods. I just don't know which one to use.

Some of the links I've been looking at:
Android App with multiple views - Best Practices?
Android - Activities vs Views
Android Activities and Views
Android: What is better - multiple activities or switching views manually?

EDIT: More detail about App structure and code samples included

GameActivity:

/*IMPORT STATEMENTS REMOVED*/

public class GameActivity extends Activity implements OnTouchListener {

@Override
protected void onCreate(Bundle savedInstanceState) {

    super.onCreate(savedInstanceState);
    setContentView(R.layout.game);  
    Log.d("LOGCAT", "GameActivity Started");

    //get the world_name from MenuActivity
    Intent intent = getIntent();
    String world_name = intent.getStringExtra(MenuActivity.EXTRA_MESSAGE);

    //load the world
    World world = loadWorld(world_name);

    //create a tilemap and get the tile translator array from it 
            //need to convert this to a static Map
    TileMap tileMap = new TileMap(this);    
    Map<Integer,Bitmap> tileTranslator = tileMap.getTileTranslator();

    //Create a reference to the MapView object and set the translator
    MapView mapView = (MapView) findViewById(R.id.map_view);
    mapView.setArgs(world, tileTranslator);

    //implement the OnTouchSwipeListener
    mapView.setOnTouchListener(new OnSwipeTouchListener() {

            /*CODE REMOVED*/
    });

}

@Override
public boolean onOptionsItemSelected(MenuItem item) {
    switch (item.getItemId()) {
    case android.R.id.home:
        NavUtils.navigateUpFromSameTask(this);
        return true;
    }
    return super.onOptionsItemSelected(item);
}

public World loadWorld(String world_name) {

//Create a dummy world to load into - why?!
    World dummy_world = new World();

    //load the world 
    Log.d("LOGCAT", "Loading the World");
    try {
        World world = dummy_world.loadWorld(this, world_name);
        return world;
    } catch (IOException e) {
    //do nothing!
    } catch (ClassNotFoundException f) {
    //do nothing!
    }

    return dummy_world; //if world load fails, send back the default world
                        // NOTE: it's not saved!!!

}

}

MapView:

/*IMPORT STATEMENTS REMOVED*/

public class MapView extends View implements OnClickListener {

protected Context context;
public World world;
public Map<Integer,Bitmap> tileTranslator;

    //hardcoded variables for testing
private int tile_width = 50;
private int tile_height = 50;
public int screen_width = 12;
public int screen_height = 6;
public int playerX = 4;
public int playerY = 7;


public MapView(Context context, AttributeSet attrs) {

    super(context, attrs);
    this.context = context;
    Log.d("LOGCAT", "MapView created"); 

    setOnClickListener(this);

}

@Override
public void onDraw(Canvas canvas) {

        /*CODE REMOVED*/
    }

//ugly method, need to break it out into individual setters
public void setArgs(World world, Map<Integer,Bitmap> tileTranslator){

    this.world = world;
    this.tileTranslator = tileTranslator;

}

}

World:

/*IMPORT STATEMENTS REMOVED*/

public class World implements Serializable {

public String world_name;
public int world_width;
public int world_height;
public int[][] world_map;

public World() { //default world - I don't even want this constructor here!

    world_name = "default_world";
    world_width = 1;
    world_height = 1;

    world_map = createWorld(world_width, world_height);

}

public World(String world_name, int world_width, int world_height) {

    //set the world attributes
    this.world_name = world_name;
    this.world_width = world_width;
    this.world_height = world_height;

    //generate the map
    world_map = createWorld(world_width, world_height);

}

private int[][] createWorld(int world_width, int world_height) {

    //create a local tile map 
    int[][] world_map = new int[world_width][world_height];

    //get a randomizer to fill the array with - {temporary solution}
    Random rand = new Random();

    //fill the tile map array with random numbers between 0 and 2
    for(int row = 0; row < world_map.length; row++) {
        for (int col = 0; col < world_map[row].length; col++) {
            world_map[row][col] = rand.nextInt(3);  //static number, needs variable!
                                                    //3 is the number of tile types
        }
    }

    return world_map;

}   

public void saveWorld(Context context, String world_name, World world) throws IOException {

    FileOutputStream fos = context.openFileOutput(world_name, Context.MODE_PRIVATE);
    ObjectOutputStream oos = new ObjectOutputStream(fos);
    oos.writeObject(world);
    oos.close();

}

public World loadWorld(Context context, String world_name) throws IOException, ClassNotFoundException {

    FileInputStream fis = context.openFileInput(world_name);
    ObjectInputStream ois = new ObjectInputStream(fis);
    World world = (World)ois.readObject();

    /*this.world_name = world.world_name;
    this.world_width = world.world_width;
    this.world_height = world.world_height;
    this.world_map = world.world_map;*/  //why doesn't this work?
    return world;

}

}

Some code removed to save space. Let me know if any of the removed code or other Activities would be useful to see.

More details about what's going on behind the scenes:

The App starts with a MenuActivity, which has 2 buttons that each lead to another activity: WorldGenerationActivity or WorldSelectionActivity.

In WorldGenerationActivity, a user is shown a TextEdit screen and a button. They enter in parameters for the world they want to generate (world_name, world_width, world_height). Once they click the button, a World object is created and saved to a file using the given world_name as the file name. The file saving is done via a saveWorld(String world_name) method available in the World class. The instance of the newly created World object has it's saveWorld() method called, the file is saved, and the user is kicked back to the parent MenuActivity via a call to finish().

In WorldSelectionActivity, the user is shown a ListView that is plugged into an ArrayAdapter. An array of filenames is created from the files contained in the world saving directory, and the adapter enables the listview to display these filenames in a list. The user selects one and the selection is sent back to the parentMenuActivity as a string via an intent.putExtra(). WorldSelectionActivity is started for result, so it ends and we are back to MenuActivity.

Once MenuActivity gets the result back from WorldSelectionActivity, it stores the putExtra() message in a parameter and then calls GameActivity. It sends the message on to GameActivity via another putExtra().

GameActivity receives the message and stores it in a variable called world_name. It then creates a World object and passes the world_name string to the loadWorld(String world_name) method of the World class, which loads the specific World requested by the user earlier from a previous file save. I sort of glossed over how this is handled in my earlier explanation. Since I need a World object to load the world into, I am forced to first create a dummy World object in GameActivity, and then call its loadWorld method, and pass the result into yet another newly created World object. This resulted in my having to include a constructor with no parameters in the World class that I didn't want there. I'm not sure why I couldn't get it to work without created a dummy World first. I tried putting the file reading logic into the no-parameter constructor, but that didn't seem to work either. I think I'm missing something here but that's not my biggest concern at this point.

MapView is one of the views contained in GameView. The login in its onDraw() method requires information from a World object. I used to have all the World loading and constructing done here, and back then I only needed to create a World once and do whatever I wanted with it. Once I moved the loadWorld() method out of MapView and into the World class itself, and also moved the calling of that method out of MapView and into GameActivity, it seemed I was suddenly creating temporary World objects all over the place. I'm wondering if there is a cleaner way to go about this and still keep things in the classes that make sense.

Answer 1

I think your version 3 is really better than the 2 others: your model (i.e. World) is independent from your your view (MapView) and your controller (GameActivity) bind them together.

I think you can improve the way your World object is created using the Builder pattern so that the job to create it is in a separate class. Let me show you what I mean :

public class WorldBuilder {

    private File worldFile;
    private String name = "default_world";
    private int width = 1;
    private int height = 1;

    public static WorldBuilder fromFile(File worldFile){
        WorldBuilder worldBuilder = new WorldBuilder();
        worldBuilder.worldFile = worldFile;
        return worldBuilder;
    }

    public WorldBuilder withName(String name){
        this.name= name;
        return this;
    }

    public WorldBuilder withWidth(int width){
        this.parameter2 = param2;
        return this;
    }

    public WorldBuilder withHeight(int height){
        this.height = height;
        return this;
    }

    public World build(){
        World world = new World(name,width,height);
        if(worldFile!=null)
            world.loadWorld(worldFile);
        return world;
    }    
}

In GameActivity, you can create the world with this single line of code:

World world = WorldBuilder.fromFile(worldFile)
        .withName(p1)
        .withWidth(p2)
        .withHeight(p3)
        .build();

And if you need to create a world with the default parameters you can simply write :

World world = WorldBuilder.fromFile(null).build();

EDIT

Where writing the code ?

All computation code than relies only on World data can be written in the World class. Never pass the MapView as an argument of a World method (keep the model independent from the view).

Try, as much as possible, to organize your code in such a way that computation is not done in MapView. The MapView must only contains code directly related to display.