Application to calculate scores of strings submitted to an endpoint

Question

Recently submitted a code assessment and I was wondering what other's feedback might be. What red flags can you see? I included gradle file if you're interested in running the application locally. Note that I used H2 to make it easier for the grader to run the app without having to worry about the DB they need install and configure. The expected input is of this sort ...

<string>,<name> 

Examples:

aauaiibbbru,Ivaana Bello 
eiiuuegiebeici,Cami Diaz 
eiiegbczu,Bob Bloch

Added README file for more clarity and details. Here is the code.

Application.java

package com.nr.stringville;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
public class Application {

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

}

SubmissionController.java

package com.nr.stringville.controllers;

import com.google.common.base.CharMatcher;
import com.nr.stringville.models.Formula;
import com.nr.stringville.models.Submission;
import com.nr.stringville.repositories.SubmissionRepository;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;

@RestController
public class SubmissionController {
    public static final int TOP_LIST_SIZE = 10;
    private final AtomicInteger validSubsCount = new AtomicInteger(0);
    private final AtomicInteger invalidSubsCount = new AtomicInteger(0);
    private final LocalDateTime startTime = LocalDateTime.now();
    private final Formula formula = new Formula();
    private volatile Submission lastSubmission = null;
    private final DateTimeFormatter formatter = DateTimeFormatter.ofPattern("HH:mm:ss  dd-MM-yyyy ");

    @Autowired
    private SubmissionRepository submissionRepository;

    @RequestMapping("/")
    public String index() {
        return "Welcome to Stringville!";
    }

    @PostMapping("/submission")
    public ResponseEntity submission(@RequestBody String body) {
        // Verify the length (10,000 characters) and format (ASCII) of the String
        if (body.length() > 10000) {
            getInvalidSubsCount().getAndIncrement();
            return ResponseEntity.status(400).body("Input too long");
        }
        if (!CharMatcher.ascii().matchesAllOf(body)) {
            getInvalidSubsCount().getAndIncrement();
            return ResponseEntity.status(400).body("Input contains invalid characters");
        }

        String[] parts = body.split(",");

        // Validate string as having the comma, that the string.length>=1 and that name.length>=1
        if (parts.length == 1) {
            getInvalidSubsCount().getAndIncrement();
            return ResponseEntity.status(400).body("Missing information");
        }

        if (parts[0].trim().length() < 1) {
            getInvalidSubsCount().getAndIncrement();
            return ResponseEntity.status(400).body("Invalid submission");
        }

        if (parts[1].trim().length() < 1) {
            getInvalidSubsCount().getAndIncrement();
            return ResponseEntity.status(400).body("Invalid name");
        }
        // Valid input. Proceed to save in the DB
        getValidSubsCount().getAndIncrement();
        Submission submission = new Submission(parts[1],parts[0],calculateScore(parts[0]));
        try {
            submissionRepository.save(submission);
        } catch (Exception exception) {
            exception.printStackTrace();
            return ResponseEntity.status(400).body("Unable to persist submission. Verify there is no submission already recorded for this name");
        }

        // Record as last submission
        setLastSubmission(submission);
        return ResponseEntity.status(200).body("submission accepted");
    }

    @GetMapping("/results")
    public ResponseEntity results() {
        StringBuilder response = new StringBuilder("submission accepted");
        List<Submission> submissions = submissionRepository.findAll();

        // Sort results to be able to display top TOP_LIST_SIZE only
        submissions.sort(Collections.reverseOrder());
        if (submissions.size() > 0) {
            response = new StringBuilder();
            // Limit the result size to the least of TOP_LIST_SIZE or records count
            int maxRecords = Math.min(submissions.size(),TOP_LIST_SIZE);
            for (int i = 0; i < maxRecords; i++) {
                response.append(submissions.get(i).getName()).append(",").append(submissions.get(i).getScore());
                // Add a line break if not at the end of the list
                if (i < maxRecords - 1) {
                    response.append("\n");
                }
            }
        }
        return ResponseEntity.status(200).body(response.toString());
    }

    @GetMapping("/health")
    public ResponseEntity health() {
        StringBuilder response = new StringBuilder();
        response.append("System uptime : ").append(calculateTimeSince(startTime)).append("\n");
        response.append("Valid submissions : ").append(getValidSubsCount()).append("\n");
        response.append("Invalid submissions : ").append(getInvalidSubsCount()).append("\n");
        if (getValidSubsCount().intValue() > 0  && getLastSubmission() != null) {
            response.append("Last submission : ").append(getLastSubmission().getName()).append(" at ")
                    .append(getLastSubmission().getTime().format(formatter)).append("(")
                    .append(calculateTimeSince(getLastSubmission().getTime()))
                    .append(" ago)").append("\n");
        } else {
            response.append("Last submission : No submissions yet." + "\n");
        }
        return ResponseEntity.status(200).body(response.toString());
    }

    @GetMapping("/reset")
    public ResponseEntity reset() {
        getValidSubsCount().set(0);
        getInvalidSubsCount().set(0);
        setLastSubmission(null);
        submissionRepository.deleteAll();
        return ResponseEntity.status(200).body("system reset");
    }

    public AtomicInteger getValidSubsCount() {
        return validSubsCount;
    }
    public AtomicInteger getInvalidSubsCount() {
        return invalidSubsCount;
    }

    public void setValidSubsCount(int value) {
        this.validSubsCount.set(value);
    }

    public void setInvalidSubsCount(int value) {
        this.invalidSubsCount.set(value);
    }

    public void setSubmissionRepository(SubmissionRepository submissionRepository) {
        this.submissionRepository = submissionRepository;
    }

    public int calculateScore(String string) {
        Map<Character,Integer> charCountMap = new HashMap<>();
        // Update count of occurrences
        for (Character character : string.toCharArray()) {
            charCountMap.merge(character, 1, Integer::sum);
        }
        return formula.getScore(charCountMap);
    }

    public String calculateTimeSince(LocalDateTime startTime) {
        StringBuilder result = new StringBuilder();
        long secondsElapsed = startTime.until(LocalDateTime.now(), ChronoUnit.MILLIS);
        long diffSeconds = secondsElapsed / 1000 % 60;
        long diffMinutes = secondsElapsed / (60 * 1000) % 60;
        long diffHours = secondsElapsed / (60 * 60 * 1000);
        long diffDays = secondsElapsed / (24 * 60 * 60 * 1000);
        if (diffDays != 0) {
            result.append(diffDays).append(" days, ");
        }

        if (diffHours != 0) {
            result.append(diffHours).append(" hours, ");
        }

        if (diffMinutes > 1) {
            result.append(diffMinutes).append(" minutes, ");
        } else if (diffMinutes > 0) {
            result.append(diffMinutes).append(" minute, ");
        }

        if (diffSeconds != 0) {
            result.append(diffSeconds).append(" seconds");
        }
        if (result.toString().isEmpty()) {
            result.append("Just now");
        }
        return result.toString();
    }

    public Submission getLastSubmission() {
        return lastSubmission;
    }

    public void setLastSubmission(Submission lastSubmission) {
        this.lastSubmission = lastSubmission;
    }
}

submission.java

package com.nr.stringville.models;

import javax.persistence.*;
import java.time.LocalDateTime;

@Entity
public class Submission implements Comparable<Submission>{

    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    private Long id;

    @Column(unique = true)
    private String name;
    private String string;
    private LocalDateTime time;
    private int score;

    public Submission(){
    }

    public Submission(String name, String string, int score){
        this.name = name;
        this.string = string;
        this.time = LocalDateTime.now();
        this.score = score;
    }

    public void setId(Long id) {
        this.id = id;
    }

    public String getString() {
        return string;
    }

    public String getName() {
        return name;
    }

    public LocalDateTime getTime() {
        return time;
    }

    public void setTime(LocalDateTime time) {
        this.time = time;
    }

    public int getScore() {
        return score;
    }

    public void setScore(int score) {
        this.score = score;
    }

    @Override
    public int compareTo(Submission o) {
        return score - o.score;
    }
}

Formula.java

package com.nr.stringville.models;

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

public class Formula {
    private final List<Rule> ruleList = new ArrayList<>();
    private static final String EXACT = "exactly";
    private static final String EVERY = "every";
    public Formula() {
        // This can be offloaded to json file, or a db for runtime customization not requiring compiling
        this.ruleList.add(new Rule('a',EXACT,3,1));
        this.ruleList.add(new Rule('e',EXACT,2,5));
        this.ruleList.add(new Rule('i',EVERY,2,2));
        this.ruleList.add(new Rule('g',EXACT,1,3));
        this.ruleList.add(new Rule('u',EVERY,1,1));
        this.ruleList.add(new Rule('z',EVERY,1,-10));
    }

    public int getScore(Map<Character,Integer> charCountMap) {
        int result = 0;
        for (Rule rule : ruleList) {
            Integer count = charCountMap.get(rule.character);
            if (count == null || count == 0) {
                // Don't do anything, just safety against null. Keeping this here for clarity
            } else if (EXACT.equals(rule.getFrequency()) &&   count.equals(rule.getCount())) {
                result = result + rule.getPoints();
            } else if (EVERY.equals(rule.getFrequency())) {
                result = result +  Math.floorDiv(count,rule.getCount()) * rule.getPoints();
            }
        }
        return result;
    }

    static class Rule {
        private final char character;
        private final String frequency;
        private final int count;
        private final int points;

        public Rule(char character, String frequency, int count, int points) {
            this.character = character;
            this.frequency = frequency;
            this.count = count;
            this.points = points;
        }

        public String getFrequency() {
            return frequency;
        }

        public int getCount() {
            return count;
        }

        public int getPoints() {
            return points;
        }
    }
}

submissionrepository.java

package com.nr.stringville.repositories;

import com.nr.stringville.models.Submission;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

@Repository
public interface SubmissionRepository extends JpaRepository<Submission, Long> {

}

build.gradle

buildscript {
    repositories {
        mavenCentral()
    }
    dependencies {
        classpath("org.springframework.boot:spring-boot-gradle-plugin:2.1.3.RELEASE")
    }
}

apply plugin: 'java'
apply plugin: 'idea'
apply plugin: 'org.springframework.boot'
apply plugin: 'io.spring.dependency-management'

bootJar {
    baseName = 'stringville-boot'
    version = '0.1'
}

repositories {
    mavenCentral()
}

sourceCompatibility = 1.8
targetCompatibility = 1.8

dependencies {
    implementation 'org.springframework.boot:spring-boot-starter-test'
    compile("org.springframework.boot:spring-boot-starter-web")
    runtimeOnly('com.h2database:h2')
    implementation("org.springframework.boot:spring-boot-starter-data-jpa")
    compile group: 'com.google.guava', name: 'guava', version: '23.5-jre'
}

README.md

Stringville Raffle Contest The city of Stringville organizes one of the most bizarre raffles in the country. Stringville citizens enter the competition by writing down their favorite string on a piece of paper along with their name, and putting this piece of paper in a big jar.

Unlike traditional raffles, Stringville's raffle rules are different. Instead of a single winner, there are multiple winners. Winners are selected based on a formula that is unknown to participants. At the end of the raffle the top N tickets with the highest scores are the winners.

For this year, the Stringville Raffle committee has privately agreed on the following formula. A ticket will be given:

+1 point if the string contains exactly three a's

+5 points if the string contains exactly two e's

+2 points for every two i's in the string

+3 points if the string contains exactly one g

+1 point for every u in the string

-10 points for every z in the string Note: All strings are case-insensitive and characters in the string do not need to be consecutive to get points. Participants can submit more than 1 string.

Here are a few example scores:

Raffle Ticket: aaaiibbbru,Ivaana Bello

Three a's (+1), two i's (+2), one u (+1) = 4 points

Raffle Ticket: eiiegiebeici,Camille Diaz

Two i's (+2), two i's (+2), one g (+3) = 7 points

Raffle Ticket: eiiegbczu,Bob Bloch

Two e's (+5), two i's (+2), one g (+3), one z (-10), one u (+1) = 1 point

Raffle Ticket: aaaluckycharmz,Derek Lee

One u (+1), one z (-10) = -9 point

Raffle Ticket: bananaram,Bonnie Chang

0 points

In this example, Camille, Ivaana, and Bob would be the lucky winners.

Stringville Raffle Service As a Software Engineer selected by Stringville to help automate this process, your task is to use this starter project to do the following:

Build an endpoint at /submission that will accept POST requests

The body of each submission will be plain text, containing the citizen's string followed by a comma and then their name. The payload (string,name) must not exceed 10,000 characters (ASCII only). The string must be at least one character. The name must be present.

Sample ticket: abekdkbjbjeheimaaabb,James LaCroix

If the ticket doesn't conform to the submission rules, then the endpoint should return a 400 status code with a message body explaining the reason why it wasn't accepted. Otherwise the endpoint should return a 200 status code indicating that the submission was accepted.

Build an endpoint at /results that will accept GET requests. The endpoint will display the top 10 winners based on the scoring system above.

The endpoint should display a list of the top 10 winners and their scores. Each winner should be on a new line with their name followed by their score (separated by a comma).

Example:

Cami Diaz,9 Ivaana Bello,5 Bob Bloch,1 ... Build an endpoint at /health that will accept GET requests. This endpoint should help you monitor your system. This should display any necessary information to know if your system is working as expected, including any useful statistics to monitor the health of your system. At a minimum, it should report the number of valid and invalid submissions.

Build an endpoint at /reset that will clear any saved or cached data, reset counts, results, and health checks.

Write unit tests that would help you validate your system. Don't forget to document any test cases you would have covered if you had more time.

Make your solution as performant as possible. The Stringville committee is interested in processing as many requests as possible. Describe any tools or scripts you used to measure performance, and what you measured (like requests per second), and any changes you made to make your solution more performant, if any.

Note:

Please use any dependency or libraries that you like. Your solution must run on port 8080 (unless documented in your instructions), and respond to /submission, /results, /health, and /reset as specified in 1-4. Please include comments where you think it's necessary. Feel free to provide a write-up of anything you would have done differently with more time, or any efforts related to performance testing. Provided A spring-boot gradle project is already stubbed out with a working test.

There is a run script run.sh which can start the application and a test script test.sh which shows basic examples of what requests look like. Feel free to edit these or create new scripts for testing purposes.

Our Evaluation We care not only about your solution, but also your general development approach. In addition to evaluating your solution for correctness and performance, we also heavily weigh your code structure and style.

Answer 1

Lot of code so I'm just going to concentrate on the interesting bits: the scoring.

Your Rule class is written specifically to the limitations of the requirements and nothing else and so it allows no easy expansions. For example, if you wanted to give points for having certain consecutive characters you would have to redesign and rewrite everything in the scoring system.

The Formula class is written so that it places quite a burden on the caller. They have to first calculate a character frequency map from the submission before they can pass the submission to the formula. This exposes the implementation of the rules to callers and thus makes changing them hard because changes to the rules now require changes to the caller too. The formula should only require the submission string as input.

As it is now, the Formula class rejects the idea of dependency injection and instead creates the Rule instances itself. This is a big red flag considering that you're using Spring Boot, which is built on providing extremely capable DI features.

Before you make optinizations like the character frequency map you should be able to show that it provides a considerable performance advantage over a more generic and more maintainable code. If you have those numbers, add them to the question. If you don't have them, you have fallen for premature optimization and failed an evaluation.

Answer 2

Bug / Misfeature

The task description states

Participants can submit more than 1 string.

However due to the unique constraint on the name field, this is prevented (unless the "participant" uses unique names for each submission).

This also removes a different problem in the code: In order to catch duplicate names you are catching (and swallowing) all Exceptions in general. Never do this. Always only catch explicitly the exceptions you are expecting. There is so much that can go wrong when accessing a database, so this way you are ignoring any other possible errors.

And you are counting a submission as valid even if the submission fails to be stored due to a duplicate name.

Also printing the stack trace is not appropriate error handling in production code, especially in a web application where no one ever will see it.

Spring Data

There are several places where you could optimize things by taking advantage of the database.

You shouldn't be storing the valid/invalid counts and the last submission in the controller. The count of valid submissions is the number of submissions in the database (especially ), so you can access that with submissionRepository.count() and the last submission can be accessed by declaring a method in SubmissionRepository:

public Submission findFirstByOrderByTimeDesc();

You shouldn't declare the class Submission as Comparable for several reasons:

• it's business logic that doesn't belong in an database entity
• it limits you to being able to sort by only one aspect
• you aren't taking advantage of the sorting features of the database

Instead you can either declare another method in SubmissionRepository:

public List<Submission> findTop10ByOrderByScoreDesc();

or, if you don't want to hard-code the number of winners into the method name, the findAll() method optionally takes a Pagination object:

Page<Submission> submissions = submissionRepository.findAll(PageRequest.of(0, 10, Sort.by(Sort.Direction.DESC, "score")));

(More possibly later)