What I Learned from Reviewing My Code from 9 Years Ago

What I Learned from Reviewing My Code from 9 Years Ago

- 8 mins

Back in 2010, I was developing games for a gaming (as in gambling) company. The games were developed in C++ using Visual Studio, and also we had products in Visual Basic .Net and C#. At that time, I was heavily influenced in the way of coding those games: lots of procedural progamming and a very strict coding style.

Being honest I think it worked really well for the kind of software we were building, but not when applied to Android development.

I wanted to create an Android app because I got me a second hand HTC G1, and I was excited to learn that the development kit for it was free. I also was addicted to a simple solitaire game called Accordion so I decided to create a game to play it on the go.

I sent the first working APK to my partner via email, so she could try it as well. Going through old emails, I was able to find the old APK but not the source code. I knew I could extract the code from it.


Rescuing the Java Code

The steps I took to rescue the code were (on Mac OS):

  1. Unzip the APK
  2. Decompile the classes.dex to Java

To unzip an APK, simply rename it to end with .zip, then use your favorite extracting tool. I used the command unzip.

Secondly, I installed jadx using Brew: brew install jadx.

With jadx, I only had to run jadx classes.dex on the unzipped APK folder.

And I got all the code I wanted. You can find the whole source code here


Reviewing the Code

This was not the first time I tried Android but was the first try I seriously tried to build something. And also, it was the first time I was coding in my free time since I started Computer Science in 2002. I didn’t know any other Android developer nor I was part of any community of developers. It is very interesting to see how this has evolved in the last decade.

The app was structured in only six classes:

The Accordion Activity contained both the MainThread and the RenderView and connected both together. The game state was stored in the Globals including an instance of the Deck. The RenderView and the MainThread accessed these globals to render and manage the game logic.

The obvious architecture issue we see here is the use of a static global to store the game logic. This will work as far the process is not destroyed by the operating system. Leaving the game for a while (like to answer a call) may result in the process being destroyed and the game progress lost.

public class Globals {
    public static Deck deck;
    public static int idx_previous_selected_card;
    public static int idx_selected_card;
    public static int logic_state;
}

The game state was small enough that it could have been stored via onSaveInstanceState and as well, rather than having a Globals class accessed from everywhere, it could have been a “Game State” model that could have been passed to the game rendering logic.

Better yet, this game state could have been stored in a local database, which would have enabled the user to save and load previous games.

On the other hand, not having stored the game state inside the rendering thread or the game management logic was a positive surprise.


The code style is also something to comment on. I mixed the code style I used at work with what the Android world was doing at that time.

The code style I used at work followed these two rules:

  1. Variables use snake_case
  2. Local variables start with underscore _local_var

But I also mixed in the camelCase code style from Java, and some mHungarian notation from Android.

See the three in action here:

for (int _idx_card = 0; _idx_card < Globals.deck.total_cards; _idx_card++) {
    Bitmap bitmapOrg = Globals.deck.deck[_idx_card].getBitmap(this);
    int x = ((_idx_card % 13) * 36) + 2;
    int y = ((_idx_card / 13) * 65) + 2;
canvas.drawBitmap(bitmapOrg, (float) x, (float) y, this.mTextPaint);

The RenderView was a nice way to separate all UI from the Activity, and I am happy I did that!

I did not use any layout, instead, I set the content view to be the RenderView directly.

public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    this.render_view = new RenderView(this);
    setContentView(this.render_view);
    this.main_thread = new MainThread();
    this.main_thread.setRenderView(this.render_view);
    this.main_thread.start();
}

Deck and Card are clearly influenced by Object Oriented Programming, two Java objects with their own methods.

Positive surprise: Card was immutable. It defines both the suit and value of a card and they were both set to be final.

Deck on the other hand, was not. And it contained two methods that modified the object internally. I learned to appreciate immutability in recent years.

In the end, the Deck was just a list of Card and the whole object could have been reduced to a simple List<Card>.

public void shuffle() {
    for (int i = 51; i > 0; i--) {
        int rand = (int) (Math.random() * ((double) (i + 1)));
        Card temp = this.deck[i];
        this.deck[i] = this.deck[rand];
        this.deck[rand] = temp;
    }
    this.total_cards = 52;
}

I don’t even need to implement a shuffle method by hand like I did, as Kotlin provides a convenient shuffle.


Let’s talk about that MainThread class.

That was the most strange thing I found in my code, but I understand why: that’s how I implemented games back then.

public class MainThread extends Thread {
    public int FPS = 10;
    public Boolean exit = Boolean.valueOf(false);
    RenderView render_view;

    public void run() {
        Globals.logic_state = 0;
        while (!this.exit.booleanValue()) {
            MainThreadRun();
        }
    }
    //...

The MainThread is a Thread looping at 10 FPS, and calling invalidate on the RenderView to force a redraw.

This was absolutely unnecessary. Simply calling to invalidate after a player movement would have been enough.


Summary

This introspection was very rewarding, looking at code from the past and realizing how far we have come has been an interesting experience that I recommend everyone to try.

I found positive how I separated the UI from the Activity, and how each component was in charge of something, methods are relatively short and the dependencies are clearly defined.

I also liked how the data model was defined and the use of Object Oriented Programming to define each model object responsibility and methods.

I didn’t like that the code style was heavily mixed, nowadays I appreciate more than ever a consistent, clean, code style.

The global statics were a bad idea, not terrible, but with consecuences that were not easy to see for someone who just got started with Android.

The MainThread, besides having a confusing name, was a very bad idea and absolutely not necessary.

My next goal is to reimplement the game from scratch. Maybe you will see a download link pretty soon :-)

Miguel Beltran

Miguel Beltran

Freelance Software Developer (Android, iOS, Flutter)

rss facebook twitter github youtube mail spotify instagram linkedin google google-plus pinterest medium vimeo stackoverflow reddit quora