Skip to content

Using Driver class to call main implementation is in LowerCasing#193

Open
Sandeep-1-Kumar wants to merge 4 commits intokscanne:masterfrom
Sandeep-1-Kumar:Code_Review_Lab
Open

Using Driver class to call main implementation is in LowerCasing#193
Sandeep-1-Kumar wants to merge 4 commits intokscanne:masterfrom
Sandeep-1-Kumar:Code_Review_Lab

Conversation

@Sandeep-1-Kumar
Copy link
Copy Markdown

Please validate and accept the pull request to submit assignment

public static void main(String[] args)
{

}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would it be better if we can avoid this extra main method?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes ! I have removed it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

great!

String returningValue = word;
if(language.equals("tr") || language.equals("az"))
{
returningValue = word.replace('I' ,'\u0131');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would it be better if we can explain what '\u0131' explains?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Absolutely ! Thanks for the suggestion.

{
if (word.endsWith("Σ"))
{
returningValue = word.substring(0, word.length() - 1) + "\u03c2";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would it be better if we can explain what '\u03c2' explains?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Absolutely ! Thanks for the suggestion.

{
if((word.charAt(0) == 'n' || word.charAt(0) == 't'))
{
char[] vowelArray = {'A','E','I','O'};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can you confirm this as all test cases are not being handled uppercase Irish vowel (A,E,I,O,U,Á,É,Í,Ó,Ú),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes , Thanks I have added the Irish vowel cases.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

great!

}
else if(language.equals("el"))
{
if (word.endsWith("Σ"))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ΠΌΛΗΣΠΌΛΗΣ el-GR ---> πόλησπόλης
this test case fails

Copy link
Copy Markdown
Author

@Sandeep-1-Kumar Sandeep-1-Kumar Feb 22, 2023

Choose a reason for hiding this comment

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

img-2

I can clearly see that the response is fine please check the screen shot and confirm.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

here, I hope check is implemented for lang "el" but not for all lang belonging to type "el", i.e "el-GR"

}
else
{
returningValue = word.replace("Σ", "σ");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ΠΌΛΗΣΠΌΛΗΣ el-GR ---> πόλησπόλης
hope this test case fails

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This test haven't failed and working fine the issue is with utf-8 visualization.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

here, I hope check is implemented for lang "el" but not for all lang belonging to type "el", i.e "el-GR"

returningValue = word.replace("Σ", "σ");
}
}
else if(language.equals("ga"))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

hope this test case is not handled : word : "nÓg" language "ga-IE"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

image
The taste case is working fine , Please verify

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

here, I hope the check is implemented for lang "ga" but not for all lang belonging to type "ga", i.e "ga-IE"


public String lowerCaseConversion(String languageType,String word)
{
String[] exceptedLanguages = {"zh" , "ja" , "th"};
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe a hashmap in case this list gets long?

}
}
String returningValue = word;
if(language.startsWith("tr") || language.startsWith("az"))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Some language codes have three letters, so "startswith" won't work here.

String returningValue = word;
if(language.startsWith("tr") || language.startsWith("az"))
{ //->'/u0131' is a Unicode representation of the lowercase dotless I.
returningValue =word.replaceAll("[iI]", "\u0131");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This doesn't look like the right behavior. Did you add tests for all combinations of these characters?

@@ -0,0 +1,70 @@
import static org.junit.Assert.*;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Good job with these. Could add more edge cases, but this is a very good start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants