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

Java program that can compile, execute and match output of given code

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

Problem

I have written code that can compile, execute, generate output and match that output with the actual output of the given code, kind of like this site. Currently, this supports only Java and C++ code. I need some suggestions about design, OO, architecture, packaging or any other suggestion that I could improve on.

This is main method from where the application start:

@Component
public class App {

  @Autowired
  QueueService queue;

  @Autowired
  Compiler compiler;

  @Autowired
  VerdictService verdictService;

  public static void main( String[] args ) {
      AbstractApplicationContext context = new AnnotationConfigApplicationContext(AppConfig.class);
      App app = context.getBean(App.class);
      app.task();
}

  private void task() {
     while (true) {
          if (!queue.isEmpty()) {
              Submission submission = queue.get();
              Verdict verdict = compiler.submit(submission); 
              verdictService.post(verdict);
          }
      }
  }
}


The compiler object takes a submission object that is a hibernate entity class. It contains the source code and has a relation with problem entity that contains the input file and the actual output of that problem. The latter generated output from the given code will match with that actual output whether the submitted code is right or wrong.

I have used lombok for the getter and setter:

@Entity
public class Submission {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Column(name = "submission_id")
private long id;

@Column(name = "username")
private String userName;

@Column(name = "status")
private boolean status;

@ManyToOne(optional = false)
@JoinColumn(name = "problem_id", referencedColumnName = "problem_id")
private Problem problem;

@Column(name = "language")
private String lang;

@Column(name = "src_file", length = 100000)
private byte[] srcFile;

@Column(name = "time")
@Temporal(TemporalType.TIMESTAMP)
private Date time;
}


Problem entity:

```
@Entity
pu

Solution

Just a few thoughts, this is by no means a complete review.

Variable naming

-
In CompilerImpl you have variables like

private String WORKING_DIR;
private String FILE_NAME_CPP;


and so on. The (albeit rather old) code conventions suggest that
this case should be only used for constants, not for regular
variables.

-
In public String getSrcName(boolean var); the meaning of var is
so unobvious that you felt like commenting it yourself. Why not name
it something like boolean withExtension?

Potential Nullpointers

See e.g. ProcessBuilderFactory.getProcessBuilder:

public static ProcessBuilder getProcessBuilder(DTO dto) {
    ProcessBuilder processBuilder = null;
    switch (dto.getLang()) {
        case Language.CPP :
            processBuilder = new ProcessBuilder("g++", dto.getSrcName(true), "-o", dto.getSrcName(false));
            break;
        case Language.JAVA :
            processBuilder = new ProcessBuilder("javac", dto.getSrcName(true));
            break;
    }
    processBuilder.directory(new File(dto.getWorkingDir()));
    return processBuilder;
}


As far as I can see dto.getLang() comes directly from the input submission (not fully shown), so there is no guarantee that it will be either Language.CPP or Language.JAVA, leaving processBuilder potentially null. Since dto.getLang() returns a String I would suggest to replace it with an enum and put some validation before accepting a submission so that valid values are guaranteed during processing.

Superfluous variables

The following struck my eye because flow is actually superfluous:

boolean flow = true;
CompileStatus status;
ProcessBuilder compile = ProcessBuilderFactory.getProcessBuilder(dto);
ProcessBuilder execute = ProcessBuilderFactory.getExecutionProcessBuilder(dto);

status = engine.compile(compile);
if (status != CompileStatus.COMPILE_SUCCESS) //CompileStatus is enum class
    flow = false;

if (flow) {
    status = engine.execute(execute, dto);
    if (status != CompileStatus.EXECUTION_SUCCESS)
        flow = false;
}

if (flow) {
    status = diffCheck();
    logger.info("Diff checker, {}", status);
}


Why not implement it like this?

ProcessBuilder compile = ProcessBuilderFactory.getProcessBuilder(dto);
ProcessBuilder execute = ProcessBuilderFactory.getExecutionProcessBuilder(dto);

CompileStatus status = engine.compile(compile);

if (status == CompileStatus.COMPILE_SUCCESS) {
    status = engine.execute(execute, dto);
}

if (status == CompileStatus.EXECUTION_SUCCESS) {
    status = diffCheck();
    logger.info("Diff checker, {}", status);
}

Code Snippets

private String WORKING_DIR;
private String FILE_NAME_CPP;
public static ProcessBuilder getProcessBuilder(DTO dto) {
    ProcessBuilder processBuilder = null;
    switch (dto.getLang()) {
        case Language.CPP :
            processBuilder = new ProcessBuilder("g++", dto.getSrcName(true), "-o", dto.getSrcName(false));
            break;
        case Language.JAVA :
            processBuilder = new ProcessBuilder("javac", dto.getSrcName(true));
            break;
    }
    processBuilder.directory(new File(dto.getWorkingDir()));
    return processBuilder;
}
boolean flow = true;
CompileStatus status;
ProcessBuilder compile = ProcessBuilderFactory.getProcessBuilder(dto);
ProcessBuilder execute = ProcessBuilderFactory.getExecutionProcessBuilder(dto);

status = engine.compile(compile);
if (status != CompileStatus.COMPILE_SUCCESS) //CompileStatus is enum class
    flow = false;

if (flow) {
    status = engine.execute(execute, dto);
    if (status != CompileStatus.EXECUTION_SUCCESS)
        flow = false;
}

if (flow) {
    status = diffCheck();
    logger.info("Diff checker, {}", status);
}
ProcessBuilder compile = ProcessBuilderFactory.getProcessBuilder(dto);
ProcessBuilder execute = ProcessBuilderFactory.getExecutionProcessBuilder(dto);

CompileStatus status = engine.compile(compile);

if (status == CompileStatus.COMPILE_SUCCESS) {
    status = engine.execute(execute, dto);
}

if (status == CompileStatus.EXECUTION_SUCCESS) {
    status = diffCheck();
    logger.info("Diff checker, {}", status);
}

Context

StackExchange Code Review Q#139778, answer score: 3

Revisions (0)

No revisions yet.