1
\$\begingroup\$

So I am working on a project that should take .txt file as an input and analyze words in it (assuming all words are English). Program must run asynchronously. Final output should be multiple .txt files, each with words and their count, arranged alphabetically. And there should be four files for words starting AG, HN, OU, VZ.

I wrote the program, but to be honest I am not happy with it, the structure looks messy. But I can not come up with how to improve it. Also I will add test later. I am a bit more confident about writing them, so won't bother you with that part of the code.

I've also added some comments and links on which I based some my solutions.

I would really appreciate all suggestions and criticism.

I have one controller method:

@RequestMapping("/file-reader")
@RestController
public class FileReaderController {

    @Autowired
    InputFileService service;

    @RequestMapping(value = "/zip",
            method = RequestMethod.POST)
    public void readFile(MultipartFile file) throws IOException {

        Map<FileType, byte[]> result = service.readFile(file);

        ByteArrayOutputStream baos = new ByteArrayOutputStream();
        ZipOutputStream zipFile = new ZipOutputStream(baos);

        result.forEach((ft, content) -> {
            try {
                zipFile.putNextEntry(new ZipEntry(ft + ".txt"));
                zipFile.write(content);
                zipFile.closeEntry();
            } catch (IOException e) {
                e.printStackTrace();
            }
        });
    }
}

And Services implementations (with their interfaces accordingly).

This service is responsible for returning final result. What I am not proud of in here is creation of multiple lists, maps... But could't get my head around more sophisticated method. Also I am not sure if it is fine to create ExecutorService twice. But I've done it trying to make program as fast as possible.

@Service
public class InputFileServiceImpl implements InputFileService {

    // create output lists
    private List<String> wordsAG = new ArrayList<>();
    private List<String> wordsHN = new ArrayList<>();
    private List<String> wordsOU = new ArrayList<>();
    private List<String> wordsVZ = new ArrayList<>();

    private Map<FileType, List<String>> allocatedWordsMap = new HashMap<>();

    @Inject
    private WordsCountingService wordsCountingService;
    @Inject
    private TextAnalyzerService textAnalyzerService;

    @Override
    public Map<FileType, byte[]> readFile(@NotNull MultipartFile file) throws IOException {
        byte[] fileContent = file.getBytes();

        // create map for storing counted words (in bytes) with letter indicator
        Map<FileType, byte[]> resultContent = new HashMap<>();

        if (fileContent.length > 0) {
            String fileData = new String(fileContent).toLowerCase();
            List<String> wordsList = textAnalyzerService.splitTextIntoWords(fileData);

            // put list in map to pass to another method
            allocatedWordsMap.put(FileType.AG, wordsAG);
            allocatedWordsMap.put(FileType.HN, wordsHN);
            allocatedWordsMap.put(FileType.OU, wordsOU);
            allocatedWordsMap.put(FileType.VZ, wordsVZ);

            // in case list is big partition it to make sorting async
            if (wordsList.size() > 4000) {
                List<List<String>> smallerLists = Lists.partition(wordsList, 4);

                // https://stackoverflow.com/questions/949355/executors-newcachedthreadpool-versus-executors-newfixedthreadpool
                ExecutorService es1 = Executors.newCachedThreadPool();

                smallerLists.forEach(smallerList -> {
                    es1.execute(() -> textAnalyzerService.allocateWords(smallerList, allocatedWordsMap));
                });

                // ensure that all thread are over. https://stackoverflow.com/questions/7939257/wait-until-all-threads-finish-their-work-in-java#
                es1.shutdown();
                try {
                    es1.awaitTermination(1, TimeUnit.MINUTES);
                } catch (InterruptedException e) {
                    // TODO: add normal error
                    e.printStackTrace();
                }
            } else {
                textAnalyzerService.allocateWords(wordsList, allocatedWordsMap);
            }

            // when all words from file are analyzed sort them, count and put into map
            ExecutorService es2 = Executors.newCachedThreadPool();
            allocatedWordsMap.forEach((tp, w) -> {
                if (w.size() > 0) {
                    es2.execute(() -> {
                        resultContent.put(tp, wordsCountingService.countWords(w));
                    });
                }
            });
        } else {
            // TODO: if file empty return error?
        }

        return resultContent;
    }

Service for analyzing text. Not sure if that huge switch is good practice and smart solution or stupid one.

@Service
public class TextAnalyzerServiceImpl implements TextAnalyzerService {

    @Override
    public List<String> splitTextIntoWords(String text) {
        List<String> words = new ArrayList<>();
        BreakIterator breakIterator = BreakIterator.getWordInstance();
        breakIterator.setText(text);

        int lastIndex = breakIterator.first();
        while (BreakIterator.DONE != lastIndex) {
            int firstIndex = lastIndex;
            lastIndex = breakIterator.next();
            if (lastIndex != BreakIterator.DONE && Character.isLetterOrDigit(text.charAt(firstIndex))) {
                words.add(text.substring(firstIndex, lastIndex));
            }
        }
        return words;
    }

    @Override
    public void allocateWords(List<String> wordsList, Map<FileType, List<String>> allocatedWordsMap) {
        wordsList.forEach(word -> {
            char firstLetter = word.charAt(0);

            switch (firstLetter) {
                case 'a':
                case 'b':
                case 'c':
                case 'd':
                case 'e':
                case 'f':
                case 'g':
                    addWordToList(word, allocatedWordsMap.get(FileType.AG));
                    break;
                case 'h':
                case 'i':
                case 'j':
                case 'k':
                case 'l':
                case 'm':
                case 'n':
                    addWordToList(word, allocatedWordsMap.get(FileType.HN));
                    break;
                case 'o':
                case 'p':
                case 'r':
                case 's':
                case 't':
                case 'u':
                    addWordToList(word, allocatedWordsMap.get(FileType.OU));
                    break;
                case 'v':
                case 'z':
                    addWordToList(word, allocatedWordsMap.get(FileType.VZ));
                    break;
            }
        });
    }

    private void addWordToList(String word, List<String> wordsList) {
        wordsList.add(word);
    }
}

Service for counting words:

@Component
public class WordsCountingServiceImpl implements WordsCountingService {

    @Override
    public byte[] countWords(@NotEmpty List<String> words) {
        Map<String, Integer> countedWords = new HashMap<>();

        words.forEach(word -> {
            if (countedWords.containsKey(word)) {
                countedWords.put(word, countedWords.get(word) + 1);
            } else {
                countedWords.put(word, 0);
            }
        });
        return countedWords.toString().getBytes();
    }

Also for handling error and making them more informative. I am not very familiar with custom errors, so maybe there are better practices than @ControllerAdvice.

@ControllerAdvice
public class GlobalControllerExceptionHandler {

    private Logger log = LoggerFactory.getLogger(this.getClass());

    @ExceptionHandler(IOException.class)
    @ResponseStatus(HttpStatus.BAD_REQUEST)
    @ResponseBody
    public ErrorDTO handle(IOException e) {
        log.error(String.format("Invoked exception handler for %s", e.getClass()), e);
        return new ErrorDTO("INPUT FILE NOT FOUND", "Input file not found");
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.