6
\$\begingroup\$

I have been doing a brilliant course on coursera based on the book "The Elements of Computing Systems: Building a Modern Computer from First Principles" by Simon Schocken and Noam Nisan.

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

In essence, you are given a text file containing a list of commands in the Hack Assembly Language (a very simplified fictitious assembly language) and you are to convert it to binary, storing the result in another file.

Please advise me on how I can make this better, more efficient, readable, maintainable etc.

Here is my implementation:

import java.io.*;
import java.lang.*;
import java.util.regex.*;
import java.util.HashMap;

public class Main {
    private String file;
    private int pc = 0;

    public Main( String fin ) {
        file = fin;
    }

    public void main( String[] args ) throws IOException, Exception {  
        // CREATE INSTANCES OF OTHER MODULES
        Parser fp = new Parser( args[ 0 ] );
        Parser sp = new Parser( args[ 0 ] );
        Code code = new Code();
        HashMap<String, String> st = new HashMap<String, String>();

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

        // FIRST PASS
        fp.advance();
        while( fp.command != null ) {   
            if( fp.commandType() == "L_COMMAND" ) {
                st.put( fp.symbol(), Integer.toString( pc ) );
                pc--;
            }
            fp.advance();
            pc++;
        }

        // SECOND PASS
        FileWriter writer = null;
        int rAllocation = 16; // Keeps a record of the last register allocated to a variable.

        try {
            // CREATE FILE, FILE WRITER
            File nf = new File( file.replaceAll( "\\.asm", ".hack" ) );
            nf.createNewFile();
            writer = new FileWriter( nf );

            // SECOND PASS
            sp.advance();
            while( sp.command != null ) {
                if( sp.commandType() == "L_COMMAND" ) {
                    // Do nothing.
                } else if( sp.commandType() == "A_COMMAND" ) {
                    if( !( Pattern.compile( "[a-zA-Z]" ).matcher( sp.symbol() ).find() ) ) { // If the symbol consists of only digits.
                        writer.write( convertAddr( sp.symbol() ) + "\n" ); // Translate integer value to binary, write to file.
                    } else if( st.get( sp.symbol() ) == null ){
                        st.put( sp.symbol(), Integer.toString( rAllocation ) ); // Assign the variable an unoccupied register. 
                        rAllocation++;
                        writer.write( convertAddr( st.get( sp.symbol() ) ) + "\n" );  // Retrieve the just allocated value from SymbolTable, translate to binary, write.
                    } else {
                        writer.write( convertAddr( st.get( sp.symbol() ) ) + "\n" );  // Retrieve value of symbol from SymbolTable, translate to binary, write.
                    }
                } else if( sp.commandType() == "C_COMMAND" ) {
                    String d = code.dest( sp.dest() );
                    String c = code.comp( sp.comp() );
                    String j = code.jump( sp.jump() );

                    writer.write( "111" + c + d + j + "\n" );
                }
                sp.advance();
            }
        } catch( IOException e ) {
            e.printStackTrace();
        } finally {
            // CLOSE WRITER
            writer.flush();
            writer.close();
        }
    }

    private String convertAddr( String addr ) throws Exception{
        String bin;
        String zeroPad = "";
        if( addr != null ) {
            bin = Integer.toBinaryString( Integer.parseInt( addr ) );
            for( int i = 0; i < ( 16 - bin.length() ); i++ ) {
                zeroPad += "0";
            }
            return zeroPad + bin; 
        } else {
            throw new Exception( "Null Parameter." );
        }
    }
}

import java.util.HashMap;

public class Code {
    // Create HashMaps
    HashMap< String, String > cMap = new HashMap< String, String >();
    HashMap< String, String > dMap = new HashMap< String, String >();
    HashMap< String, String > jMap = new HashMap< String, String >();

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

        // Populate the Dest Mnemonics HashMap
        dMap.put( "", "000" ); dMap.put( "M", "001" ); dMap.put( "D", "010" );
        dMap.put( "MD", "011" ); dMap.put( "A", "100" ); dMap.put( "AM", "101" );
        dMap.put( "AD", "110" ); dMap.put( "AMD", "111" );

        // Populate the Jump Mnemonics HashMap
        jMap.put( "", "000" ); jMap.put( "JGT", "001" ); jMap.put( "JEQ", "010" );
        jMap.put( "JGE", "011" ); jMap.put( "JLT", "100" ); jMap.put( "JNE", "101" );
        jMap.put( "JLE", "110" ); jMap.put( "JMP", "111" );
    }

    public String dest( String d ) {
        // Return the corresponding binary string or null, if the mnemonic is invalid.
        if( dMap.get( d ) != null ) return dMap.get( d );
        return null;
    }

    public String comp( String c ) {
        if( cMap.get( c ) != null ) return cMap.get( c );
        return null;
    }

    public String jump( String j ) {
        if( jMap.get( j ) != null ) return jMap.get( j );
        return null;
    }
}

import java.io.*;
import java.util.regex.*;
import java.util.HashMap;

public class Parser {
    private BufferedReader br;
    String command;

    public Parser( String file ) {
        try {
            br = new BufferedReader( new FileReader( file ) );
        } catch ( Exception e ) {
            e.printStackTrace();
        }
    }

    public void advance() throws IOException {      
        String line;       
        while( true ) {
            line = br.readLine();
            if( line == null ) {
                try {
                    if( br != null ) {
                        br.close();
                        command = null;
                    }
                } catch( IOException ex ) {
                    ex.printStackTrace();
                } finally {   
                    break;
                }
            }
            line = line.replaceAll( "\\s","" ).replaceAll( "//.*", "" );
            if( line.length() != 0 ) {
                command = line;
                break;
            }
        }
    }

    public String commandType() {
        if( command.charAt( 0 ) == '(' ) {
            return "L_COMMAND";
        } else if( command.charAt( 0 ) == '@' ) {
            return "A_COMMAND";
        } else {
            return "C_COMMAND";
        }
    }

    public String symbol() {
        if( command.indexOf( "@" ) == -1 ) {
            return command.replaceAll( "\\(", "" ).replaceAll( "\\)", "" );
        } else {
            return command.replaceAll( "@", "" );
        }
    }

    public String comp() {
        return command.replaceAll( ".*=", "" ).replaceAll( ";.*", "" );
    }

    public String dest(){
        if( command.indexOf( "=" ) == -1 ) {
            return "";
        } else {
            return command.replaceAll( "=.*", "" );
        }
    }

    public String jump() {
        if( command.indexOf( ";" ) == -1 ) {
            return "";
        } else {
            return command.replaceAll( ".*;", "" );
        }
    }
}
\$\endgroup\$

1 Answer 1

Reset to default
4
\$\begingroup\$

Here some comments about your code:

  1. You should never compare Strings by using ==.

    if( sp.commandType() == "A_COMMAND" )

    This might work for constant Strings if the compiler pools the constants but this is not guaranteed. So use .equals(..) instead

  1. You should use an Enum for L_COMMAND A_COMMAND C_COMMAND. This way you gain type safety and also speed because Enums are mapped to int by the compiler which makes comparisons very cheap.
  1. You use .replaceAll a lot during parsing. This results in creating new String instances and always iterating through the String. If speed is you goal you should try to avoid to modify the line during parsing. A better way would be to only read the source and generate a data structure while parsing it (often done by a tokenizer and creating a abstract syntax tree). Another benefit would be that you can always say where in the original line you are when a parsing problem occurs because you never modified the line.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.