patternjavaMinor

JavaCC Android port User Interface (Activity) class

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

androiduserinterfaceportjavaccactivityclass

Problem

I have been working on a port of JavaCC for Android. It is mainly an interface. I needed only to modify the original source to redirect output to a TextView. The point of this project is to rekindle my coding skills after years of dormancy, to actually finish a coding project, and to make a useful port of JavaCC that I can use to play around with AIDE. This project was actually made in part with AIDE. I am excited to read your opinion on my code regarding what I consider to be the most difficult portion of the code. I was able to modularize many portions of this UI package, but this class persisted to be difficult. Please feel free to ask any applicable questions.

```
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintStream;

import android.app.Activity;
import android.content.Context;
import android.os.Bundle;
import android.os.Handler;
import android.os.Message;
import android.text.Editable;
import android.text.InputType;
import android.text.TextWatcher;
import android.view.View;
import android.view.ViewGroup.LayoutParams;
import android.widget.Button;
import android.widget.CheckBox;
import android.widget.EditText;
import android.widget.LinearLayout;
import android.widget.ScrollView;
import android.widget.TextView;

public class MainActivity extends Activity {
public static Context context;
private String[] menuOptions = new String[] { "view license",
"view parameters", "run program", "view output" };
private Runnable[] menuOptionThreads = new Runnable[] { new Runnable() {
@Override
public void run() {
try {
println(getLicenseString().toString(), handler);
} catch (IOException e) {
println("Error reading license file.", handler);
}
scrolly.removeAllViews();
scrolly.addView(text);
}
}, new Runnable() {

@Override
public void run() {
scrolly.removeAl

Solution

private String[] menuOptions = new String[] { "view license",
        "view parameters", "run program", "view output" };
private Runnable[] menuOptionThreads = new Runnable[] { new Runnable() {
    @Override
    public void run() {
        try {
            println(getLicenseString().toString(), handler);
        } catch (IOException e) {
            println("Error reading license file.", handler);
        }
        scrolly.removeAllViews();
        scrolly.addView(text);
    }
}, new Runnable() {

    @Override
    public void run() {
        scrolly.removeAllViews();
        scrolly.addView(linear);
    }

}, new Runnable() {

    @Override
    public void run() {

        text.setText("");
        scrolly.removeAllViews();
        scrolly.addView(text);
        new Run(handler, options, fPath, fOutPath);
    }

}, new Runnable() {

    @Override
    public void run() {

        scrolly.removeAllViews();
        scrolly.addView(text);
    }

} };

So, there's an abstract "menu option" of sorts that has a String and Runnable.

Turn it into a class.

You could use some blank lines in your code. onCreate reads as a wall of text, and without semantic breaks ("paragraphs"), it's hard to see at a glance what's going on.

Additionally, you'll find that blank lines show how a function would be split up into other functions.

Let's take a look at your code:

@Override
public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    context = this;
    Streams.error = new PrintStream(new Streams.ErrorStream(handler));
    Streams.standard = new PrintStream(new Streams.StandardStream(handler));
    LinearLayout rootView = new LinearLayout(this);
    rootView.setOrientation(LinearLayout.VERTICAL);
    scrolly = new ScrollView(this);
    linear = new LinearLayout(this);
    linear.setOrientation(LinearLayout.VERTICAL);
    try {
        fPath = getIntent().getDataString();
        fPath = fPath.substring(fPath.indexOf("//") + 2);
    } catch (NullPointerException e) {
        fPath = "/";
    }
    text = new TextView(this);
    options = Option.generateDefaultOptionsObjectsArray();

    Button menuButton = new Button(this);
    menuButton.setText("menu");
    new AppMenu(context, menuOptions, menuButton, MainActivity.this,
            menuOptionThreads);
    Button fileSelectButton = new Button(this);
    fileSelectButton.setText("file");
    new FileSelect(context, new File("/").list(), fileSelectButton,
            MainActivity.this, "/", false);
    path = new EditText(this);
    path.setText(fPath);
    path.addTextChangedListener(new TextWatcher() {
        @Override
        public void beforeTextChanged(CharSequence p1, int p2, int p3,
                int p4) {
        }

        @Override
        public void onTextChanged(CharSequence p1, int p2, int p3, int p4) {
        }

        @Override
        public void afterTextChanged(Editable arg0) {
            fPath = arg0.toString();
            int dot = fPath.lastIndexOf(".");
                if (dot > 0) outPath.setText(fPath.substring(0, dot));
        }
    });
    outPath = new EditText(this);
    outPath.addTextChangedListener(new TextWatcher() {

        @Override
        public void afterTextChanged(Editable arg0) {
            // TODO Auto-generated method stub

        }

        @Override
        public void beforeTextChanged(CharSequence arg0, int arg1,
                int arg2, int arg3) {
            // TODO Auto-generated method stub

        }

        @Override
        public void onTextChanged(CharSequence arg0, int arg1, int arg2,
                int arg3) {
            fOutPath = arg0.toString();
        }

    });
    LinearLayout outPathOuterView = new LinearLayout(this);
    TextView outPathLabel = new TextView(this);
    outPathLabel.setText(Option.OUTPUT_DIRECTORY_OPTION);
    outPathOuterView.addView(outPathLabel);
    outPathOuterView.addView(outPath);
    linear.addView(outPathOuterView);
    for (Option option : options)
        linear.addView(generateViewForOption(option));
    LinearLayout topLayout = new LinearLayout(this);
    topLayout.addView(menuButton);
    topLayout.addView(fileSelectButton);
    topLayout.addView(path);
    rootView.addView(topLayout);
    scrolly.addView(linear);
    rootView.addView(scrolly);
    setContentView(rootView);
}

I'd start by sorting relevant lines.

fPath is defined as some value in a try catch, but then not used until 10 statements later. By sorting the statements you can say "this region handles fPath, that region handles..." I would give examples, but the code is a bit confusing right now.

Lastly, once you have blank lines, I'd suggest migrating whole paragraphs to separate functions. The function as a whole is too long. A good start would be migrating outPath and path out of the onCreate function, as the listeners take a lot of vertical space.

```
if (option.getType().equals(Option.Type.INTEGER)) {
EditText value = new

Code Snippets

private String[] menuOptions = new String[] { "view license",
        "view parameters", "run program", "view output" };
private Runnable[] menuOptionThreads = new Runnable[] { new Runnable() {
    @Override
    public void run() {
        try {
            println(getLicenseString().toString(), handler);
        } catch (IOException e) {
            println("Error reading license file.", handler);
        }
        scrolly.removeAllViews();
        scrolly.addView(text);
    }
}, new Runnable() {

    @Override
    public void run() {
        scrolly.removeAllViews();
        scrolly.addView(linear);
    }

}, new Runnable() {

    @Override
    public void run() {

        text.setText("");
        scrolly.removeAllViews();
        scrolly.addView(text);
        new Run(handler, options, fPath, fOutPath);
    }

}, new Runnable() {

    @Override
    public void run() {

        scrolly.removeAllViews();
        scrolly.addView(text);
    }

} };

@Override
public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    context = this;
    Streams.error = new PrintStream(new Streams.ErrorStream(handler));
    Streams.standard = new PrintStream(new Streams.StandardStream(handler));
    LinearLayout rootView = new LinearLayout(this);
    rootView.setOrientation(LinearLayout.VERTICAL);
    scrolly = new ScrollView(this);
    linear = new LinearLayout(this);
    linear.setOrientation(LinearLayout.VERTICAL);
    try {
        fPath = getIntent().getDataString();
        fPath = fPath.substring(fPath.indexOf("//") + 2);
    } catch (NullPointerException e) {
        fPath = "/";
    }
    text = new TextView(this);
    options = Option.generateDefaultOptionsObjectsArray();

    Button menuButton = new Button(this);
    menuButton.setText("menu");
    new AppMenu(context, menuOptions, menuButton, MainActivity.this,
            menuOptionThreads);
    Button fileSelectButton = new Button(this);
    fileSelectButton.setText("file");
    new FileSelect(context, new File("/").list(), fileSelectButton,
            MainActivity.this, "/", false);
    path = new EditText(this);
    path.setText(fPath);
    path.addTextChangedListener(new TextWatcher() {
        @Override
        public void beforeTextChanged(CharSequence p1, int p2, int p3,
                int p4) {
        }

        @Override
        public void onTextChanged(CharSequence p1, int p2, int p3, int p4) {
        }

        @Override
        public void afterTextChanged(Editable arg0) {
            fPath = arg0.toString();
            int dot = fPath.lastIndexOf(".");
                if (dot > 0) outPath.setText(fPath.substring(0, dot));
        }
    });
    outPath = new EditText(this);
    outPath.addTextChangedListener(new TextWatcher() {

        @Override
        public void afterTextChanged(Editable arg0) {
            // TODO Auto-generated method stub

        }

        @Override
        public void beforeTextChanged(CharSequence arg0, int arg1,
                int arg2, int arg3) {
            // TODO Auto-generated method stub

        }

        @Override
        public void onTextChanged(CharSequence arg0, int arg1, int arg2,
                int arg3) {
            fOutPath = arg0.toString();
        }

    });
    LinearLayout outPathOuterView = new LinearLayout(this);
    TextView outPathLabel = new TextView(this);
    outPathLabel.setText(Option.OUTPUT_DIRECTORY_OPTION);
    outPathOuterView.addView(outPathLabel);
    outPathOuterView.addView(outPath);
    linear.addView(outPathOuterView);
    for (Option option : options)
        linear.addView(generateViewForOption(option));
    LinearLayout topLayout = new LinearLayout(this);
    topLayout.addView(menuButton);
    topLayout.addView(fileSelectButton);
    topLayout.addView(path);
    rootView.addView(topLayout);
    scrolly.addView(linear);
    rootView.addView(scrolly);
    setContentView(rootView);
}

if (option.getType().equals(Option.Type.INTEGER)) {
        EditText value = new EditText(this);
        value.setInputType(InputType.TYPE_CLASS_NUMBER);
        value.setText(String.valueOf(option.getInt()));
        option.setValueView(value);
        result.addView(value);
    } else if (option.getType().equals(Option.Type.BOOLEAN)) {
        CheckBox value = new CheckBox(this);
        value.setChecked(Boolean.valueOf(option.getBool()));
        option.setValueView(value);
        result.addView(value);
    } else if (option.getType().equals(Option.Type.STRING_OPTIONLISTABLE)) {
        Button value = new Button(this);
        value.setText(option.getCurrentValueAsString());
        new ParameterPopup(context, option.getPossibleOptions(), value);
        option.setValueView(value);
        result.addView(value);
    } else if (option.getType().equals(Option.Type.STRING)) {
        EditText value = new EditText(this);
        value.setText(option.getCurrentValueAsString());
        option.setValueView(value);
        result.addView(value)

Context

StackExchange Code Review Q#70266, answer score: 2

Revisions (0)

No revisions yet.