Skip to content

Convert to LowerCase ( code review lab)#194

Open
MVR9999 wants to merge 4 commits intokscanne:masterfrom
MVR9999:feature
Open

Convert to LowerCase ( code review lab)#194
MVR9999 wants to merge 4 commits intokscanne:masterfrom
MVR9999:feature

Conversation

@MVR9999
Copy link

@MVR9999 MVR9999 commented Feb 16, 2023

AllTestCasesToLowerCase.java file has all the test cases. ConvertToLowerCase.java, we give input and expect the output result.txt has all the tested test cases

AllTestCasesToLowerCase.java file has all the test cases.
ConvertToLowerCase.java, we give input and expect the output
result.txt has all the tested test cases

public class ToLowerCase {

String turkish_lang = "tr";

Choose a reason for hiding this comment

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

I feel that naming conventions for attribute names in camelCase looks better rather than separating with "_"
Ex: turkish_lang to turkishLang

Copy link
Author

Choose a reason for hiding this comment

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

noted!

String result = object.wordToLowerCase(words[i],lang[i],langWithNoLowerCase);
printWriter.println(words[i]+ " " + lang[i] +" "+result);
}

Copy link

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

Choose a reason for hiding this comment

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

I can suggest that elimination of unused extra empty lines, It might make code looks messy. There are a lot in the code.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, we shall handle the indentation and the line spacing

}
else if(language.startsWith(irish_lang))
{
if((word.charAt(0) == lowerCase_n || word.charAt(0) == lowerCase_t) &&

Choose a reason for hiding this comment

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

A better logic for this implementation is to use for loop , defining the possibilities in an array. As you can see that if else multiple conditions is not an good way of sorting.

Copy link
Author

Choose a reason for hiding this comment

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

noted! we shall implement this better using a for loop

return lower1;
}

boolean isLanguage_with_no_lowerCase(String []langWithNoLowerCase,String language)

Choose a reason for hiding this comment

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

I felt that in the above method , method name looks good than here as it uses camel casing. You can code better following single naming convection for all the methods in the class.

import java.io.UnsupportedEncodingException;
import java.util.*;

public class ToLowerCase {

Choose a reason for hiding this comment

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

I think in java a class name should be the file name they should match. However we know that we can just have a single class in single .java file , Its a good way to name the class after file.

"NATHAIR", "tOthair", "tETHAIR", "tITHAIR", "nÓg", "nÕg","对不起","ごめんなさい", "KASIM", "KASIM", "ΠΌΛΗΣ", "官话", "ภาษาไทย", "ΠΌΛΗΣ", "ΠΌΛΗΣΠΌΛΗΣ"};

// words in their respective languages
String lang[] = {"en", "en-US", "en-IE", "en-Latn", "ga", "ga", "ga", "ga", "ga",

Choose a reason for hiding this comment

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

I think when one value in an array satisfies condition duplicates are not necessary in array -> "ga".


PrintWriter printWriter = null;
try {
printWriter = new PrintWriter("D:\\SLU\\Spring 23\\PSD\\result.txt", "UTF-8");

Choose a reason for hiding this comment

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

I think the path can be the same directory where class is present because Path fluctuates and creates mess on different systems if you run. printWriter - > path

@MVR9999
Copy link
Author

MVR9999 commented Feb 22, 2023

Hello, I have modified the code as per the given suggestions and added the JUnit test cases, Could you please confirm

@Sandeep-1-Kumar
Copy link

Sign Off all verified.

String irishLang = "ga";
String latinDotlessI = "\u0131";

String upperCaseI = "I";
Copy link
Owner

Choose a reason for hiding this comment

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

These three maybe unnecessary? I'm happy with a literal 't' for example.


import java.util.*;

public class ConvertToLowerCase {
Copy link
Owner

Choose a reason for hiding this comment

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

I don't like the repeated code here and in AllTestCasesToLower....

} }
}

return lower1;
Copy link
Owner

Choose a reason for hiding this comment

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

No Greek?

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.

3 participants