Randomly Generate Planets

Question

This is code I'm writing for an android game where I want to be able to randomly generate small planet sprites. It seems like the best way to do this is to have a set of small PNG textures that can be layered onto each other. I found code for recolouring a drawable programmatically here that was very helpful.

The idea is that the Planet object will randomly choose attributes on creation. This makes it easier to save the data rather than having the Drawables themselves stored as part of the object. My intent is to have a backlog of these saved so that by the end of the game I can redraw all the ones you've seen. So data storage and later retrieval is important.

Once the Planet has been created then there's the getDrawable method that can be called to actually return a LayerDrawable object, which layers the textures onto each other as you'd expect.

I'd like to know particularly if I'm performing right and matching Android coding style. I don't Java or Android terribly well so quite possibly I have some bad code smell plopped in here. In particular, how should I be commenting? Python has docstrings (programmatically accessible comments with a clear format guideline) but is there any similar guideline for how to detail the functions in Java?

package com.chadwick.laika;

import android.content.Context;
import android.graphics.Bitmap;
import android.graphics.Canvas;
import android.graphics.ColorFilter;
import android.graphics.LightingColorFilter;
import android.graphics.Paint;
import android.graphics.drawable.BitmapDrawable;
import android.graphics.drawable.Drawable;
import android.graphics.drawable.LayerDrawable;
import android.support.v4.content.ContextCompat;

import java.util.Random;

public class Planet {

    private Context context;
    private Random random = new Random();
    private int[] palette;
    private String[] layers;

    public Planet(Context context){
        this.context = context;
        palette = randomPalette();
        layers = getLayers();
    }

    public int[] randomPalette() {
        String id = "palette" + random.nextInt(2);
        int palette = context.getResources().getIdentifier(id, "array", context.getPackageName());
        return context.getResources().getIntArray(palette);
    }

    // Randomly pick a base and textures to layer onto the base, then store their codes
    private String[] getLayers(){
        String[] layers = new String[random.nextInt(3) + 2];

        int base_id = random.nextInt(3);
        layers[0] = "base" + base_id;

        for (int i = 1; i < layers.length; i++){
            String code = "texture" + base_id + "_" + random.nextInt(6);
            layers[i] = code;
        }

        return layers;
    }

    // Apply color to source. source must be a purely white drawable (can have transparency)
    public Drawable colourDrawable(Drawable source, int color) {
        Bitmap bitmap = Bitmap.createBitmap(source.getIntrinsicWidth(),
                source.getIntrinsicHeight(),
                Bitmap.Config.ARGB_8888);
        Canvas canvas = new Canvas(bitmap);
        source.setBounds(0, 0, canvas.getWidth(), canvas.getHeight());
        source.draw(canvas);

        Paint p = new Paint();
        ColorFilter filter = new LightingColorFilter(color, 1);
        p.setColorFilter(filter);

        canvas.drawBitmap(bitmap, 0, 0, p);
        return new BitmapDrawable(context.getResources(), bitmap);
    }

    // Takes stored image codes and palette to generate the LayerDrawable
    public LayerDrawable getDrawable(){
        Drawable[] pictures = new Drawable[layers.length];
        int base = context.getResources().getIdentifier(layers[0], "drawable", context.getPackageName());
        pictures[0] = colourDrawable(ContextCompat.getDrawable(context, base), palette[0]);

        for (int i = 1; i < layers.length; i++) {
            int texture = context.getResources().getIdentifier(layers[1], "drawable", context.getPackageName());
            pictures[1] = colourDrawable(ContextCompat.getDrawable(context, texture), palette[i]);
        }
        return new LayerDrawable(pictures);
    }
}

Answer 1

Bug?

public LayerDrawable getDrawable(){
    Drawable[] pictures = new Drawable[layers.length];
    int base = context.getResources().getIdentifier(layers[0], "drawable", context.getPackageName());
    pictures[0] = colourDrawable(ContextCompat.getDrawable(context, base), palette[0]);

    for (int i = 1; i < layers.length; i++) {
        int texture = context.getResources().getIdentifier(layers[1], "drawable", context.getPackageName());
        pictures[1] = colourDrawable(ContextCompat.getDrawable(context, texture), palette[i]);
    }
    return new LayerDrawable(pictures);
}

What I see here is that you go through the layers, and turn it into a set of pictures. That's your intention, at least. That's why you do new Drawable[layers.length]. But then you loop through the layers and just read layers[1] and write pictures[1] and the only thing i is ever used for is palette...

If it's because of the similarity between i and 1, configure your editor to use different colors for (local) variables and numbers.

Also, that function looks like it does a lot of work. I don't know a lot about android, so maybe this is a bad thing to do, but could the drawables be cached?

Magic Numbers & Unnecessary local store

private String[] getLayers(){
    String[] layers = new String[random.nextInt(3) + 2];

    int base_id = random.nextInt(3);
    layers[0] = "base" + base_id;

    for (int i = 1; i < layers.length; i++){
        String code = "texture" + base_id + "_" + random.nextInt(6);
        layers[i] = code;
    }

    return layers;
}

You have random.nextInt with 3, then +2, and 3 somewhere else, and 6 somewhere else again...

I don't know what they mean. You have apparently 2-4 layers each but the top(?) textured with a random 0-5 subtextures... which seems oddly rigid to me, why 6 different subtextures, why not 5, why not 7...

        String code = "texture" + base_id + "_" + random.nextInt(6);
        layers[i] = code;

That's an unnecessary local store. You don't do anything with the variable, and it doesn't have a name that clarifies anything. I'd remove it, just put the following in your for loop:

layers[i] = "texture" + base_id + "_" + random.nextInt(6);