HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Implementing Multi-File Programs in Vitsy

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filemultiprogramsvitsyimplementing

Problem

I'm the proud owner of the Vitsy programming language, which I've been working on for some time (except recently, because high school). It's only been used in PPCG so far, but I hope to expand it to more useful things. It is a top-access-only 2-dimensional stack-based language that moves in multiple directions through Vitsy code with the ability to use other "classes". The only problem is that, a lot of the time, the way I have been loading these "classes" is extremely inefficient, terribly written, and shoddily thought out.

Here is an excerpt that I would like to be reviewed:

```
public static void methodHandler(int source) throws InterruptedException, IOException, ScriptException {
olddir.add(direction);
direction = true;
if (source == -1) {
oldpos.add(new Integer[]{currin,position});
currin = top().intValue();
position = 0;
rmtop();
while (position currin().length - 1) {
break;
}
else opHandle();
position = (direction) ? (position + 1): (position - 1 >= 0) ? position - 1: currin().length-1;
}
currin = oldpos.get(oldpos.size()-1)[0].intValue();
position = oldpos.get(oldpos.size()-1)[1].intValue();
oldpos.remove(oldpos.size()-1);
direction = olddir.get(olddir.size()-1);
olddir.remove(olddir.size()-1);
ending = false;
return;
}
instruct.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-1)[source]}, new boolean[]{false, false}));
users.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-1)[source]}, new boolean[]{true, false}).get(0));
extender.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-2)[source]}, new boolean[]{false, true}).get(0)[0]);
oldposext.add(new Integer[]{currin, position}

Solution

Yeah this looks like golfed code :)

It's never a pleasure to review spaghetti code. So I won't give a comprehensive review, with luck somebody else will, or we can each improve some bits and pieces. So here are a few suggestions to get you started.

Spacing

Use more whitespace in general. Not excessively, but more. Paste your code in an IDE, and use the auto-reformat feature. It will add spaces around operators, and the code will become easier to read.

Then add some vertical space. This means empty lines, to separate chunks of code that are only loosely related, and keep lines together that are closely related. Each group of lines (not separated by empty lines in between) should form a cohesive unit, doing some logical step.

As a next step, add a comment above each group of lines to explain the logical step they accomplish.

As a next step, extract reach group of lines to a helper method, converting the comment to a method name.

Other specific tips

Given this:

instruct.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-1)[source]}, new boolean[]{false, false}));
users.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-1)[source]}, new boolean[]{true, false}).get(0));
extender.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-2)[source]}, new boolean[]{false, true}).get(0)[0]);
oldposext.add(new Integer[]{currin, position});
currin = (source == -2)?currin:top().intValue();
currclassname.add((source == -2)?extender.get(extender.size()-2):users.get(users.size()-2)[source]);


If I'm looking at this right (it's hard to see in the jumble), all these lines depend on the condition source == -2. Not only it's a waste to evaluate that multiple times unnecessarily, this is very hard to read.

Instead of repeating an ugly ternary on all these lines, rewrite as a proper if-else.

Refactoring spaghetti code

It's ok to make small steps that may seem insignificant at first. As the code gets cleaner bit by bit, more and more improvement opportunities become clear, gradually leading to more substantial improvements.

My tips so far don't change the logic of your code. When you start changing the logic, later, as you make progress with cleaning, how will you know you're not breaking something? You can know (almost), by writing unit tests first, covering all execution paths. After that you can go on and refactor boldly yet fairly safely, re-running the unit tests after every refactoring step. Without automated tests, verifying the behavior using manual testing will be a nightmare.

Martin Fowler has a famous book on refactoring techniques. That should help a lot.

Code Snippets

instruct.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-1)[source]}, new boolean[]{false, false}));
users.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-1)[source]}, new boolean[]{true, false}).get(0));
extender.add(FileHandler.getFileInstruct(new String[]{(source == -2)?extender.get(extender.size()-1):users.get(users.size()-2)[source]}, new boolean[]{false, true}).get(0)[0]);
oldposext.add(new Integer[]{currin, position});
currin = (source == -2)?currin:top().intValue();
currclassname.add((source == -2)?extender.get(extender.size()-2):users.get(users.size()-2)[source]);

Context

StackExchange Code Review Q#119561, answer score: 2

Revisions (0)

No revisions yet.