I have written this code to check all the edge cases in the implemention of an atoi() function.Please review.
//Program to print an atoi(). public class Atoi{ public static int atoiFunc(String str,char[] c,int l){ //If the length is 0,the function returns 0; if(l==0){ return 0; } //variable to store the result int res = 0; //variable to check for the sign.Initially,initially with positive 1. int sign = 1; // try/catch block to check the bounds of Integer datatype. try{ int check = Integer.valueOf(str); } catch(NumberFormatException e){ System.out.println("Number is going out of bounds of integer datatype."); System.exit(0); } for(int i=0; i<l; i++){ if(c[i]=='-'){ i++; sign = -1; } if(c[i]=='+'){ i++; } if(c[i]!=' '){ res = res*10 + c[i] -'0'; } } return sign*res;}public static void main(String[] args) { String s = "-9846032"; char[] ch = s.toCharArray(); int len = s.length(); int val = atoiFunc(s,ch,len); System.out.println("Original : "+s); System.out.println("After atoi : "+val); }}- 1\$\begingroup\$I don't understand why
atoiFuncrequires you to provides, its char array, and its length when the function itself could easily get those values? Do you ever expectctonot be the char array ofs? Do you ever expectltonot be the length ofs(andch)? Why shouldatoiFuncnot just take the string?\$\endgroup\$Ron Beyer– Ron Beyer2016-11-14 20:29:07 +00:00CommentedNov 14, 2016 at 20:29 - \$\begingroup\$Also your title"I have written this code to check all the edge cases" isn't right, you are only checking a single, normal case and no edge cases.\$\endgroup\$Ron Beyer– Ron Beyer2016-11-14 20:30:52 +00:00CommentedNov 14, 2016 at 20:30
1 Answer1
String s = "-9846032";char[] ch = s.toCharArray();int len = s.length();int val = atoiFunc(s,ch,len);How come you chose to do it this way? In general, you shouldn't pass every permutation of a value into a function. If the function wants s.toCharArray, it can call it itself.
Imagine if we did the following:
String s = "-9846032";char[] ch = "abcdef".toCharArray();int len = "lololololololololol".length();int val = atoiFunc(s,ch,len);What does that even mean? To protect yourself from such nonsense and to make your API easier to understand, we should remove them. If you remove them, it means that your function can be called as such:
int val = atoiFunc("12345678")which is much more preferable.
//If the length is 0,the function returns 0;if(l==0){ return 0;}What happens ifl is-1? Or any other negative number? What happens ifl is e.g. 12? Can you convert a 12 digit number to anint?
Importantly: How does someone differentiateatoiFunc("0") fromatoiFunc("") ?
//variable to store the resultint res = 0;//variable to check for the sign.Initially,initially with positive 1.int sign = 1;"variable to" is superfluous -- we already know they're variables :)
// try/catch block to check the bounds of Integer datatype. try{ int check = Integer.valueOf(str);}catch(NumberFormatException e){ System.out.println("Number is going out of bounds of integer datatype."); System.exit(0);}What is the use in this?
Integer.valueOf converts a string to an integer. It does exactly what you're already trying to do!
So therefore I assume this exercise is some kind of study/homework? In which case relying on Integer.valueOf is a massive cop-out, and probably against the spirit of the code. I can see you're using it as a bounds check. But there are other, better ways to do that that will actually a) teach you something and b) not rely on a function that already does what your function is trying to do :)
If we go back to this comment you made:
I have written this code to check all the edge cases in the implemention of an atoi() function.Please review.
Then we can see that you haven't checked anything!checkInt does it all for you! It's basically cheating :P So please remove that part of the code and try to think up a proper way to check the conditionscheckInt is catching.
(Note: If this isn't a study piece, then the entire program should be replaced by a call to Integer.valueOf or even Integer.parseInt :))
for(int i=0; i<l; i++){almost all style guides ask for a bit of space around binary operators:
for (int i = 0; i < l; i++){It's hard to critic the loop, as I would like to say things such as "What happens if a character is a letter?", but it's hard to say that because of thecheckInt cheat in the way, which protects you. So I'll comment more after you've fixed the code to remove that!
Here are some more tests for your function, however:
assert 1234 == atoiFunc("1234") assert -1234 == atoiFunc("-1234") assert 0 == atoiFunc("0") atoiFunc("") # should raise an error atoiFunc(null) # should raise an error assert 0 == atoiFunc("+0") assert 0 == atoiFunc("-0") atoiFunc("1234abc") # should raise an error atoiFunc("abc1234") # should raise an error atoiFunc("12 34") # should raise an error assert 1234 == atoiFunc("+1234") atoiFunc("+12+34") # should raise an error atoiFunc("12-34") # should raise an error atoiFunc("1234+") # should raise an error atoiFunc("-1234+") # should raise an error atoiFunc("+1234-") # should raise an error atoiFunc("1234-") # should raise an error atoiFunc(" 1234 ") # should maybe raise an error. up to you. int x = atoiFunc("4294967296") # I'll let you figure this one out :) int y = atoiFunc("4294967297") # I'll let you figure this one out :) int z = atoiFunc("-4294967296") # I'll let you figure this one out :) int w = atoiFunc("-4294967297") # I'll let you figure this one out :)You mustlog in to answer this question.
Explore related questions
See similar questions with these tags.
