Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

I have a working implementation here alreadyalready. That one took me about 2 - 3 hours. Then I wanted to code following the best practices. This time it took me 5 - 6 hours. And I am doubtful which one is more readable. But any review is welcome in this version as well. My goal was to de-couple everything as much as I could, code to Interfaces instead of Concrete classes.

I have a working implementation here already. That one took me about 2 - 3 hours. Then I wanted to code following the best practices. This time it took me 5 - 6 hours. And I am doubtful which one is more readable. But any review is welcome in this version as well. My goal was to de-couple everything as much as I could, code to Interfaces instead of Concrete classes.

I have a working implementation here already. That one took me about 2 - 3 hours. Then I wanted to code following the best practices. This time it took me 5 - 6 hours. And I am doubtful which one is more readable. But any review is welcome in this version as well. My goal was to de-couple everything as much as I could, code to Interfaces instead of Concrete classes.

Source Link
Koray Tugay
  • 1.5k
  • 5
  • 20
  • 44

Assembler for Hack Assembly Language - Take 2

I have been reading this great book and I decided to do the Project in Chapter 5: Writing an Assembler for the Hack Assembly Language.

You can find the specification for the Hack Machine Language and the Hack Assembly Language here.

I have a working implementation here already. That one took me about 2 - 3 hours. Then I wanted to code following the best practices. This time it took me 5 - 6 hours. And I am doubtful which one is more readable. But any review is welcome in this version as well. My goal was to de-couple everything as much as I could, code to Interfaces instead of Concrete classes.

I must say that both implementations do the same thing, but this one is like 5 - 6 times longer I guess. Maybe even more.. And it is full of classes, packages, interfaces etc..

Is this really following the best practices? Do you see any dependency problems? Should I have more classes? More modularitiy?

package biz.tugay.hack.assembler.assemble;
/* User: [email protected] Date: 10/03/15 Time: 15:56 */

import java.io.IOException;

public interface InstructionFileAssembler {
    public void assembleFile() throws IOException;
}

package biz.tugay.hack.assembler.assemble;
/* User: [email protected] Date: 09/03/15 Time: 22:20 */

import biz.tugay.hack.assembler.helperutils.commentRemover.InstructionCommentRemover;
import biz.tugay.hack.assembler.instruction.Instruction;
import biz.tugay.hack.assembler.instruction.factory.InstructionFactory;
import biz.tugay.hack.assembler.instruction.instructionConverter.InstructionToBinaryConverter;

import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Scanner;

public class InstructionFileAssemblerImpl implements InstructionFileAssembler {

    private InstructionCommentRemover instructionCommentRemover;
    private InputStream inputStream;
    private InstructionFactory instructionFactory;
    private List<InstructionToBinaryConverter> instructionToBinaryConverters;

    public InstructionFileAssemblerImpl(InstructionCommentRemover instructionCommentRemover,
                                        InputStream inputStream,
                                        InstructionFactory instructionFactory,
                                        List<InstructionToBinaryConverter> instructionToBinaryConverters) {
        this.instructionCommentRemover = instructionCommentRemover;
        this.inputStream = inputStream;
        this.instructionToBinaryConverters = instructionToBinaryConverters;
        this.instructionFactory = instructionFactory;
    }

    @Override
    public void assembleFile() throws IOException {
        Scanner scanner = new Scanner(inputStream);
        while (scanner.hasNextLine()) {
            String convert = convert(scanner.nextLine());
            if(convert != null && convert.length() != 0) {
                System.out.println(convert);
            }
        }
    }

    private String convert(String instructionToConvert) {

        // Obtain the next instruction from the input stream.
        // If the instruction contains comments, remove them and trim the instruction
        if (instructionToConvert.contains(instructionCommentRemover.getCommentRule())) {
            instructionToConvert = instructionCommentRemover.removeCommentFromSingleInstruction(instructionToConvert);
        }

        // Build the instruction from using the provided factory.
        Instruction instruction = instructionFactory.buildInstruction(instructionToConvert);

        // If we have a valid instruction..
        if (instruction != null && instruction.getMnemonic() != null && instruction.getMnemonic().length() != 0) {
            return doConvert(instruction);
        }
        return null;
    }

    private String doConvert(Instruction instruction) {
        for (InstructionToBinaryConverter instructionToBinaryConverter : instructionToBinaryConverters)
            if (instruction.getInstructionType().equals(instructionToBinaryConverter.getInstructionType()))
                return instructionToBinaryConverter.getBinaryRepresentationForInstruction(instruction);
        throw new UnsupportedOperationException();
    }

}

package biz.tugay.hack.assembler.helperutils.commentRemover;
/* User: [email protected] Date: 10/03/15 Time: 14:47 */

public interface InstructionCommentRemover {

    /**
     * Removes the comment part of a single line instuction.
     * The implementing class must hardcode the comment rule.
     * @param instruction : Single instruction line.
     * @return Returns the instruction without the comment part.
     */

    public String removeCommentFromSingleInstruction(String instruction);
    public String getCommentRule();
}

package biz.tugay.hack.assembler.helperutils.commentRemover;
/* User: [email protected] Date: 10/03/15 Time: 14:47 */

public class InstructionRemoverImpl implements InstructionCommentRemover {

    private String commentRule = "//";

    @Override
    public String removeCommentFromSingleInstruction(String instruction) {
        if (instruction.contains(commentRule)) {
            instruction = instruction.split(commentRule)[0];
        }
        return instruction.trim();
    }

    @Override
    public String getCommentRule() {
        return commentRule;
    }

}

package biz.tugay.hack.assembler.helperutils;
/* User: [email protected] Date: 09/03/15 Time: 20:25 */

public class BinaryFormatter {

    public static String appendZerosToString(String stringToBeAppended) {
        char[] chars = stringToBeAppended.toCharArray();
        int zerosToAppend = 16 - chars.length;
        StringBuilder stringBuilder = new StringBuilder();
        for(int i=0;i<zerosToAppend;i++) {
            stringBuilder.append("0");
        }
        stringBuilder.append(stringToBeAppended);
        return stringBuilder.toString();
    }

}

package biz.tugay.hack.assembler.instruction.factory;

import biz.tugay.hack.assembler.instruction.HackInstructionTypeA;
import biz.tugay.hack.assembler.instruction.HackInstructionTypeC;
import biz.tugay.hack.assembler.instruction.Instruction;

/* User: [email protected] Date: 09/03/15 Time: 22:23 */
public class HackInstructionFactory implements InstructionFactory {
    @Override
    public Instruction buildInstruction(String instruction) {
        Instruction hackInstruction;
        if(instruction.startsWith("(")) {
            return null;
        }
        if(instruction.startsWith("@")) {
            hackInstruction = new HackInstructionTypeA(instruction);
        }
        else {
            hackInstruction = new HackInstructionTypeC(instruction);
        }
        return hackInstruction;
    }
}

package biz.tugay.hack.assembler.instruction.factory;
/* User: [email protected] Date: 10/03/15 Time: 15:52 */

import biz.tugay.hack.assembler.instruction.Instruction;

/**
 * An Instruction Factory implementation should return a concrete and correct @Instruction object
 */
public interface InstructionFactory {
    /**
     * The implementing method must return one of the Instructions that the
     * Assembly Language implements.
     * @param instruction The code to be used to find the matching Instruction object and creating it.
     * @return Instruction object.
     */
    Instruction buildInstruction(String instruction);
}

package biz.tugay.hack.assembler.instruction.instructionConverter;
/* User: [email protected] Date: 09/03/15 Time: 21:10 */

import biz.tugay.hack.assembler.instruction.Instruction;
import biz.tugay.hack.assembler.instruction.InstructionType;

import java.util.Map;

public interface InstructionToBinaryConverter {

    public String getBinaryRepresentationForInstruction(Instruction instruction);
    public InstructionType getInstructionType();
    public void addToLookUpMap(Map<String, Integer> map);

}


package biz.tugay.hack.assembler.instruction.instructionConverter;
/* User: [email protected] Date: 09/03/15 Time: 21:27 */

import biz.tugay.hack.assembler.helperutils.BinaryFormatter;
import biz.tugay.hack.assembler.instruction.Instruction;
import biz.tugay.hack.assembler.instruction.InstructionType;

import java.util.HashMap;
import java.util.Map;

public class InstructionToBinaryConverterInstructionAImpl implements InstructionToBinaryConverter {

    private int memoryLocationCounter = 16;
    private Map<String,Integer> lookUpMap = new HashMap<String, Integer>();
    private InstructionType instructionType = InstructionType.A_INSTRUCTION;

    {
        lookUpMap.put("SP"       ,0);
        lookUpMap.put("LCL"      ,1);
        lookUpMap.put("ARG"      ,2);
        lookUpMap.put("THIS"     ,3);
        lookUpMap.put("THAT"     ,4);
        lookUpMap.put("R0"       ,0);
        lookUpMap.put("R1"       ,1);
        lookUpMap.put("R2"       ,2);
        lookUpMap.put("R3"       ,3);
        lookUpMap.put("R4"       ,4);
        lookUpMap.put("R5"       ,5);
        lookUpMap.put("R6"       ,6);
        lookUpMap.put("R7"       ,7);
        lookUpMap.put("R8"       ,8);
        lookUpMap.put("R9"       ,9);
        lookUpMap.put("R10"      ,10);
        lookUpMap.put("R11"      ,11);
        lookUpMap.put("R12"      ,12);
        lookUpMap.put("R13"      ,13);
        lookUpMap.put("R14"      ,14);
        lookUpMap.put("R15"      ,15);
        lookUpMap.put("SCREEN"   ,16384);
        lookUpMap.put("KBD"      ,24576);
    }

    @Override
    public String getBinaryRepresentationForInstruction(Instruction instruction) {

        String mnemonic = instruction.getMnemonic();
        String symbol   = mnemonic.substring(1);

        boolean isDirectAddress = true;

        try {
            Integer.parseInt(symbol);
        } catch (NumberFormatException e) {
            isDirectAddress = false;
        }

        if(isDirectAddress) {
            return BinaryFormatter.appendZerosToString(Integer.toBinaryString(Integer.valueOf(symbol)));
        }

        if(mnemonic.startsWith("@")) {
            if(lookUpMap.containsKey(symbol)) {
                return BinaryFormatter.appendZerosToString(Integer.toBinaryString(lookUpMap.get(symbol)));
            }
            if(!lookUpMap.containsKey(symbol)) {
                lookUpMap.put(symbol,memoryLocationCounter);
                memoryLocationCounter++;
            }
            if(lookUpMap.containsKey(symbol)) {
                return BinaryFormatter.appendZerosToString(Integer.toBinaryString(lookUpMap.get(symbol)));
            }
        }

        throw new UnsupportedOperationException();

    }

    public InstructionType getInstructionType() {
        return instructionType;
    }

    @Override
    public void addToLookUpMap(Map<String, Integer> map) {
        lookUpMap.putAll(map);
    }

}

package biz.tugay.hack.assembler.instruction.instructionConverter;
/* User: [email protected] Date: 09/03/15 Time: 22:10 */

import biz.tugay.hack.assembler.instruction.InstructionType;
import biz.tugay.hack.assembler.instruction.Instruction;

import java.util.HashMap;
import java.util.Map;

public class InstructionToBinaryConverterInstructionCImpl implements InstructionToBinaryConverter {

    private Map<String,String> lookUpMap = new HashMap<String, String>();
    private Map<String,String> compMap = new HashMap<String, String>();

    @Override
    public String getBinaryRepresentationForInstruction(Instruction instruction) {

        final String mnemonic = instruction.getMnemonic();

        // We definetly have compBinary in the instruction...
        String compBinaryRepresentation         = getBinaryRepresentationForCompMnemnonic(mnemonic);

        String jumpMnemonic                     = "null";
        String jumpBinaryRepresentation;

        if (mnemonic.contains(";")) {
            jumpMnemonic = mnemonic.split(";")[mnemonic.split(";").length-1];
        }

        jumpBinaryRepresentation = lookUpMap.get(jumpMnemonic);

        String destMnemonic                     = "null";
        String destinationBinaryRepresentation;

        if(mnemonic.contains("=")) {
            destMnemonic = mnemonic.split("=")[0];
        }

        destinationBinaryRepresentation = lookUpMap.get(destMnemonic);

        return "111" + compBinaryRepresentation + destinationBinaryRepresentation + jumpBinaryRepresentation;

    }

    @Override
    public InstructionType getInstructionType() {
        return InstructionType.C_INSTRUCTION;
    }

    private String getBinaryRepresentationForCompMnemnonic(String mnemonic) {
        String compMnemonnic  = mnemonic.split(";")[0];
        compMnemonnic = compMnemonnic.split("=")[compMnemonnic.split("=").length-1];
        return compMap.get(compMnemonnic);
    }

    {
        compMap.put("0"  ,"0101010");
        compMap.put("1"  ,"0111111");
        compMap.put("-1" ,"0111010");
        compMap.put("D"  ,"0001100");
        compMap.put("A"  ,"0110000");
        compMap.put("!D" ,"0001101");
        compMap.put("!A" ,"0110001");
        compMap.put("-D" ,"0001111");
        compMap.put("-A" ,"0110011");
        compMap.put("D+1","0011111");
        compMap.put("A+1","0110111");
        compMap.put("D-1","0001110");
        compMap.put("A-1","0110010");
        compMap.put("D+A","0000010");
        compMap.put("D-A","0010011");
        compMap.put("A-D","0000111");
        compMap.put("D&A","0000000");
        compMap.put("D|A","0010101");
        compMap.put("M"  ,"1110000");
        compMap.put("!M" ,"1110001");
        compMap.put("-M" ,"1110011");
        compMap.put("M+1","1110111");
        compMap.put("M-1","1110010");
        compMap.put("D+M","1000010");
        compMap.put("D-M","1010011");
        compMap.put("M-D","1000111");
        compMap.put("D&M","1000000");
        compMap.put("D|M","1010101");
    }

    {
        lookUpMap.put("null","000");
        lookUpMap.put("JGT" ,"001");
        lookUpMap.put("JEQ" ,"010");
        lookUpMap.put("JGE" ,"011");
        lookUpMap.put("JLT" ,"100");
        lookUpMap.put("JNE" ,"101");
        lookUpMap.put("JLE" ,"110");
        lookUpMap.put("JMP" ,"111");
    }

    {
        lookUpMap.put("null","000");
        lookUpMap.put("M"   ,"001");
        lookUpMap.put("D"   ,"010");
        lookUpMap.put("MD"  ,"011");
        lookUpMap.put("A"   ,"100");
        lookUpMap.put("AM"  ,"101");
        lookUpMap.put("AD"  ,"110");
        lookUpMap.put("AMD" ,"111");
    }

    @Override
    public void addToLookUpMap(Map<String, Integer> map) {
        throw new UnsupportedOperationException("You can not mess with this instruction type!");
    }

}

package biz.tugay.hack.assembler.instruction;
/* User: [email protected] Date: 09/03/15 Time: 21:01 */

public class HackInstructionTypeA implements Instruction {

    private String mnemonic;

    public HackInstructionTypeA(String instruction) {
        this.mnemonic = instruction;
    }

    @Override
    public InstructionType getInstructionType() {
        return InstructionType.A_INSTRUCTION;
    }

    @Override
    public String getMnemonic() {
        return mnemonic;
    }

}

package biz.tugay.hack.assembler.instruction;
/* User: [email protected] Date: 09/03/15 Time: 22:08 */

public class HackInstructionTypeC implements Instruction {

    private String mnemonic;

    public HackInstructionTypeC(String instruction) {
        this.mnemonic = instruction;
    }

    @Override
    public InstructionType getInstructionType() {
        return InstructionType.C_INSTRUCTION;
    }

    @Override
    public String getMnemonic() {
        return mnemonic;
    }
}

package biz.tugay.hack.assembler.instruction;
/* User: [email protected] Date: 10/03/15 Time: 15:51 */

public interface Instruction {
    InstructionType getInstructionType();
    String getMnemonic();
}


package biz.tugay.hack.assembler.instruction;
/* User: [email protected] Date: 09/03/15 Time: 20:52 */

public enum InstructionType {
    A_INSTRUCTION,C_INSTRUCTION;
}


package biz.tugay.hack.assembler.label;
/* User: [email protected] Date: 10/03/15 Time: 14:45 */

import java.util.Map;

public interface LabelMapBuilder {
    public Map<String, Integer> buildLabelMapForGivenFile();
}


package biz.tugay.hack.assembler.label;
/* User: [email protected] Date: 09/03/15 Time: 22:36 */

import biz.tugay.hack.assembler.helperutils.commentRemover.InstructionCommentRemover;

import java.io.InputStream;
import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;

public class LabelMapBuilderImpl implements LabelMapBuilder {

    private Character labelStartCharacter;
    private InstructionCommentRemover instructionCommentRemover;
    private InputStream inputStream;

    public LabelMapBuilderImpl(InputStream inputStream,
                               InstructionCommentRemover instructionCommentRemover,
                               Character labelStartCharacter) {
        this.inputStream = inputStream;
        this.instructionCommentRemover = instructionCommentRemover;
        this.labelStartCharacter = labelStartCharacter;
    }

    @Override
    public Map<String, Integer> buildLabelMapForGivenFile() {
        int lineCounter = 0;
        Map<String, Integer> labelMap = new HashMap<String, Integer>();
        Scanner scanner = new Scanner(inputStream);
        while (scanner.hasNextLine()) {
            String instruction = scanner.nextLine();
            if(instruction.contains(instructionCommentRemover.getCommentRule())) {
                instruction = instructionCommentRemover.removeCommentFromSingleInstruction(instruction);
            }
            if (isInstructionLabel(instruction)) {
                addInstructionToLabelMap(lineCounter, labelMap, instruction);
                lineCounter--;
            }
            lineCounter++;
        }
        return labelMap;
    }

    private void addInstructionToLabelMap(int lineCounter, Map<String, Integer> labelMap, String instructionWithoutComment) {
        labelMap.put(instructionWithoutComment.substring(1, instructionWithoutComment.length() - 1), lineCounter);
    }

    private boolean isInstructionLabel(String instruction) {
        return instruction.startsWith(Character.toString(labelStartCharacter));
    }

}

and finally the Test Class:

package biz.tugay.hack.assembler;
/* User: [email protected] Date: 10/03/15 Time: 16:15 */

import biz.tugay.hack.assembler.assemble.InstructionFileAssembler;
import biz.tugay.hack.assembler.assemble.InstructionFileAssemblerImpl;
import biz.tugay.hack.assembler.helperutils.commentRemover.InstructionCommentRemover;
import biz.tugay.hack.assembler.helperutils.commentRemover.InstructionRemoverImpl;
import biz.tugay.hack.assembler.instruction.factory.HackInstructionFactory;
import biz.tugay.hack.assembler.instruction.factory.InstructionFactory;
import biz.tugay.hack.assembler.instruction.instructionConverter.InstructionToBinaryConverter;
import biz.tugay.hack.assembler.instruction.instructionConverter.InstructionToBinaryConverterInstructionAImpl;
import biz.tugay.hack.assembler.instruction.instructionConverter.InstructionToBinaryConverterInstructionCImpl;
import biz.tugay.hack.assembler.label.LabelMapBuilder;
import biz.tugay.hack.assembler.label.LabelMapBuilderImpl;

import java.io.*;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

public class TestClass {

    public static void main(String[] args) throws IOException {

        InputStream inputStream = new FileInputStream(new File(args[0]));
        InstructionCommentRemover instructionCommentRemover = new InstructionRemoverImpl();
        LabelMapBuilder labelMapBuilder = new LabelMapBuilderImpl(inputStream,instructionCommentRemover,'(');
        Map<String, Integer> labelMap = labelMapBuilder.buildLabelMapForGivenFile();

        InstructionToBinaryConverter instructionToBinaryConverterInstructionA = new InstructionToBinaryConverterInstructionAImpl();
        instructionToBinaryConverterInstructionA.addToLookUpMap(labelMap);

        InstructionToBinaryConverter instructionToBinaryConverterInstructionC = new InstructionToBinaryConverterInstructionCImpl();

        List<InstructionToBinaryConverter> instructionToBinaryConverterList = new ArrayList<InstructionToBinaryConverter>();
        instructionToBinaryConverterList.add(instructionToBinaryConverterInstructionA);
        instructionToBinaryConverterList.add(instructionToBinaryConverterInstructionC);

        InstructionFactory factory = new HackInstructionFactory();
        inputStream = new FileInputStream(new File(args[0]));

        InstructionFileAssembler instructionFileAssembler =
                new InstructionFileAssemblerImpl(instructionCommentRemover,inputStream,factory,instructionToBinaryConverterList);

        instructionFileAssembler.assembleFile();


    }

}