CPU emulator in Java

Question

I recently started new project - a CPU emulator in Java. Base code is written, now I can start to slowly implement new things, but I want code review to be done, to reformat code before it messes up. Unfortunately, I didn't add any Javadocs and didn't do any deeper tests, except checking if basic OP codes are working.

https://github.com/jakub-gonet/Emulated-CPU

Code structure is like so:

• CommandHelper.py - script used to convert Op code to binary form and vice versa
• Main - entry point
• cpu/OPCODE - list of OP codes and what they should do
• cpu/ADDR_TYPE - addressing modes
• cpu/StatusFlags - CPU flags (carry, negative, zero flag etc)
• cpu/programCounter - program counter
• cpu/CPUEmulator - main class to bind rest to it
• cpu/instructionArgument, InstructionMnemonic, Instruction - used to encode a single instruction
• cpu/instruction/InstructionRunner - fetch and run Instruction
• cpu/instruction/converter - package used to convert from bits to OP code and address modes
• cpu/memory - package used to represent CPU memory

(bolded are important)

package cpu.instruction;
import cpu.ADDR_TYPE;
import cpu.instruction.converter.ValueToAddressingMode;
import cpu.instruction.converter.ValueToOPCode;
import cpu.memory.Memory;
public class InstructionRunner {
 public static void run(Instruction instruction) {
 if (instruction != null) {
 Integer arg2Value = (instruction.getArg2() != null) ? instruction.getArg2()
 .getValue() : null;
 Integer arg1Value = (instruction.getArg1() != null) ? instruction.getArg1()
 .getValue() : null;
 Integer address = (instruction.getArg1() != null) ? instruction.getArg1()
 .getAddress() : null;
 Integer value = instruction.getMnemonic()
 .getOpcode()
 .apply(arg1Value,
 arg2Value);
 if (instruction.getArgumentCount() > 0) instruction.getArg1()
 .getMemory()
 .write(address, value);
 } else throw new
 IllegalArgumentException("ERROR: Instruction can't be null.");
 }
 public static Instruction fetchInstruction(int address) {
 try {
 int opCodeAndAddresses = readFromMemory(address);
 InstructionMnemonic mnemonic = new InstructionMnemonic(
 ValueToOPCode.getOPCode(opCodeAndAddresses)
 );
 InstructionArgument[] arguments = createArguments(mnemonic.getRequiredArgs(), address, opCodeAndAddresses);
 return new Instruction(mnemonic, arguments[0], arguments[1]);
 } catch (IndexOutOfBoundsException e) {
 System.out.println("ERROR: OP code not recognized.");
 } catch (NullPointerException e) {
 System.out.println("ERROR: Bad format of instruction.");
 }
 return null;
 }
 private static InstructionArgument[] createArguments(int argumentCount, int address, int argumentsCode) {
 InstructionArgument arg2 = null;
 InstructionArgument arg1 = null;
 switch (argumentCount) {
 case 2:
 int arg2Code = readFromMemory(address + 2);
 ADDR_TYPE secondAddressType = getAddressType(2, argumentsCode);
 arg2 = new InstructionArgument(arg2Code, secondAddressType);
 case 1:
 int arg1Code = readFromMemory(address + 1);
 ADDR_TYPE firstAddressType = getAddressType(1, argumentsCode);
 arg1 = new InstructionArgument(arg1Code, firstAddressType);
 }
 return new InstructionArgument[] {arg1, arg2};
 }
 private static ADDR_TYPE getAddressType(int argumentNumber, int operatorAndAddressingModeCode) {
 if (argumentNumber == 1) return ValueToAddressingMode.getFirstArgAddressingMode(operatorAndAddressingModeCode);
 if (argumentNumber == 2) return ValueToAddressingMode.getSecondArgAddressingMode(operatorAndAddressingModeCode);
 throw new IllegalArgumentException("ERROR: OP code argument does not exist.");
 }
 private static int readFromMemory(int address) {
 return Memory.getInstance()
 .read(address);
 }
}

I mostly wonder if that null policy is good practice and how to manage exceptions in that type of project.

• Phrancis: Thanks for adding the code. I hope you get great answers!

Answer 1

Thanks for sharing your code.

I mostly wonder if that null policy is good practice

The answer is No. Checking for null is not a good practice.

In Your case you need the null check (in the code you posted) because your code violates the most important OO principle: information hiding / encapsulation. And this is accompanied by violating the feature envy code smell.

The InstructionRunner class should not know about the internals of an Instruction. From the point of the InstructionRunner an Instruction should have an execute method that returns the new value for the instruction pointer. This execute method should get the current value of the instruction pointer, the "memory" (which is merely a list of bytes) and an object representing the ALU to be modified by the instruction.

So I'd write the code you presented this way:

 /* this is a pure data structure so public members are OK */
 class ArithmeticLogicUnit{
 public int accumulator=0;
 public int a=0; // register A
 public int b=0; // register B
 public int d=0; // register D
 public int e=0; // register E
 public int h=0; // register H
 public int l=0; // register L
 } 
 interface InstructionPool{
 //* convert byte(s)in memory at/from instructionPointer to Instruction */
 Instruction decode(int instructionPointer, List<Integer> memory);
 } 
 interface Instruction{
 //* modify the ALU and calculate the next value of <code>instructionPointer</code>*/
 int execute(int instructionPointer, List<Integer> memory, ArithmeticLogicUnit alu);
 }
 class ControlUnit{
 private final ArithmeticLogicUnit alu = new ArithmeticLogicUnit();
 private final InstructionPool instructionPool;
 private final List<Integer> memory;
 private int instructionPointer;
 public ComtrolUnit(int initialInstructionPointer, List<Integer> memory, InstructionPool instructionPool){
 this.memory = memory;
 this.instructionPool = instructionPool;
 } 
 public void run(int initialInstructionPointer){ 
 this.instructionPointer = initialInstructionPointer;
 while(instructionPointer >= 0 && instructionPointer < memory.size()){
 Instruction instruction = instructionPool.decode(instructionPointer, memory);
 instructionPointer = instruction.execute(instructionPointer, memory, alu);
 }
 }
 }

and how to manage exceptions in that type of project.

Exceptions should be handled the way as you do in any other project: Use them to interrupt the "happy day path". Use checked exceptions if you expect the calling code to recover from that error anyhow (including asking the user what to do) and use unchecked exceptions if you think the program (or at least the current thread) as a whole cannot run any longer reliably. `

• Jump3r: Should I merge ALU and registers into one thing or separate them?
• Timothy Truckle: @Jump3r In real life the ALU has the registers. So its "one thing" as I wrote it.