Skip to content
This repository has been archived by the owner on Oct 27, 2022. It is now read-only.

First skeleton of the additional coding task #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mgborowiec
Copy link
Contributor

No description provided.

Copy link

@AlexanderFroemmgen AlexanderFroemmgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot.

Two remarks (which can be addressed in a follow up pull request as well):

  1. Did you consider to move this logic into a separate class/file?
  2. What about adding some "still failing" tests early?

* @param input input string
* @return a normalized string
*/
public String normalize(String input) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparing the code with the comment, it seems like there are some cornercases which would be handled unexpected, e.g., what about unicode characters?

* @param str2 second input string
* @return distance between strings
*/
public Integer distance(String str1, String str2) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is more precise than the method name. Consider keeping both in sync, e.g.,
calcLevenshteinDistance

}

/**
* Looks for the closest match (with closest distance) in the menu for an order with typos.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment let's me assume that it only works for orders with typos, but I assume that those without typos should work as well?


/**
* Looks for the closest match (with closest distance) in the menu for an order with typos.
* @param order an order with potential typos
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo?

* @return the closest order match
*/
public String findMatch(String order) {
//TODO: asses which item on menu is closest
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo

Copy link

@wefelix wefelix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider pulling out the string related functions into a utility class and add tests.

}

@Test
public void testFindMatchSimpleTypo() {
Copy link

@quantumFeline quantumFeline Sep 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about giving more descriptive test names?.. e.g. "whitespaceMissing", "middleLetterMissing", "lastLetterMissing" etc. From my experience, it is helpful in case when some tests start failing :)

public String normalize(String input) {
String normalized = input.replaceAll("[^a-zA-Z ]" , "")
.replaceAll("\\s+", "")
.toLowerCase();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some strings to test/consider:
"Latte "
"latté"

}
// Return the menu item with smallest distance
return menu[indexMin];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When happens if you pass an empty array as your menu?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants