I have implemented a solution to this Cracking the Coding Interview question:
Write a method to replace all spaces in a string with %20. You may assume that the string has sufficient space at the end of the string to hold the additional characters, and that you are given the true length of the string. USE character array to perform this operation in place.
public static String conversion(String str, int length){
char[] strChars = str.toCharArray();
int numSpaces = 0;
for(int i = 0; i < length; i++){
if(strChars[i] == ' ')
numSpaces++;
}
int newLength = length + 2 * numSpaces;
char[] newChars = new char[newLength];
for(int i = length - 1; i >= 0; i--){
char c = strChars[i];
if(c != ' '){
newChars[i + 2 * numSpaces] = c;
}
else{
newChars[i + 2 * numSpaces] = '0';
newChars[i + 2 * numSpaces - 1] = '2';
newChars[i + 2 * numSpaces - 2] = '%';
numSpaces--;
}
}
String newString = String.valueOf(newChars);
return newString;
}
Can anyone give me any feedback on my implementation in regards of efficiency or improvements?
-
2\$\begingroup\$ Whatever your implementation is, you should ask them why they decided to let a C programmer design their Java interview questions. A Java string cannot have space at the end of the string. Java strings are not modified in-place. Therefore the whole question doesn't make sense. \$\endgroup\$Roland Illig– Roland Illig2017年06月30日 17:54:46 +00:00Commented Jun 30, 2017 at 17:54
-
2\$\begingroup\$ And by the way: don't trust that book. I just filed several bug reports against its "solutions". \$\endgroup\$Roland Illig– Roland Illig2017年06月30日 19:41:17 +00:00Commented Jun 30, 2017 at 19:41
5 Answers 5
Your strategy of counting the spaces and then back-looping to shift the characters right (and replace spaces with %20
) is good. The basic algorithm is probably as good as it gets as a character array system.
Your variable names are decent, and the code flows well.
On the other hand, there are some small things I would change.
Possible bug
Your code, given the input conversion("abc ", 3)
you would output "abc"
but you should not remove any "extra" padding in the string, you should return "abc "
In fact, you should only really have the one char[]
array. The second one is making you do bad things ;-)
Enhanced fors
Use enhanced-for loops when possible, and always use {}
blocks for if-statements, even 1-liners:
for(int i = 0; i < length; i++){ if(strChars[i] == ' ') numSpaces++; }
should be:
for (char c : strChars) {
if (c == ' ') {
numSpaces++;
}
}
Comments
Comment unusual loops - your second for-loop is an odd one, and it often helps the reader if you comment why a loop is non-sequential (or even if you just make sure they are aware of it).
multiple indexes
Have you considered having a separate index for each position in the array - the position of the source character, and the position of where to insert it?
public static String conversion(String str, int length){
char[] strChars = str.toCharArray();
int numSpaces = 0;
for (int i = 0; i < length; i++) {
if(strChars[i] == ' ') {
numSpaces++;
}
}
int insert = length + 2 * numSpaces - 1;
// loop backwards through source characters.
for (int i = length - 1; i >= 0; i--) {
char c = strChars[i];
if (c == ' '){
strChars[insert--] = '0';
strChars[insert--] = '2';
strChars[insert--] = '%';
} else {
strChars[insert--] = c;
}
}
return String.valueOf(strChars);
}
The benefit of two indexes is that you can keep the logic more readable ... ("more" is relative) as each index moves by one character at a time... and a space counts as 3 characters.
-
\$\begingroup\$ A pleasure to read . And solves the problem as reuired in place :] Nitpick: I do not like the "hungarian" notations like
strChars
andnumSpaces
. But TO came up with this in the first place. \$\endgroup\$Thomas Junk– Thomas Junk2017年03月03日 19:24:45 +00:00Commented Mar 3, 2017 at 19:24 -
\$\begingroup\$ I have made it a habit to not include {} on one liners but working on it, thanks for the reminder. Also good point on the multiple indexes. \$\endgroup\$TheLearner– TheLearner2017年03月03日 20:14:11 +00:00Commented Mar 3, 2017 at 20:14
You have re-invented the wheel which makes for a rather poor answer to an interview question (Academic context would be different). A good interview answer emphasises knowledge of the inbuilt capabilities balanced with an appreciation of development priorities. Simple code that uses inbuilt libraries is quick to code, robust, widely understandable and maintainable. I would expect to see something like:
log.info(new String("/A A/B B/C C").replaceAll(" ", "%20"));
Even better would be the following proving an appreciation of Test driven development:
@Test
public void test() {
final String actualResult = new String("/A /B /C /D ").replaceAll(" ", "%20");
assertEquals("/A%20/B%20/C%20/D%20", actualResult);
}
Otherwise your coding practice is reasonable.
- You have used Java naming conventions. +1
- You have mostly named for the problem domain. +1
- Your code is readable. +1
Expanding on Interview aspect The ability to stick closely to the requirements is an important skill in a developer but should not mean blindly following them. The specification is a reflection of requirements. Requirements shouldn't specify implementation details and may be in error. Spotting bogus things and having the strength of character to call them out in a constructive manner are important skills in a developer. A skilled interviewer can also use coding questions to test your reaction and behaviour as well as your technical/coding skills. As an interviewer I will often ask the impossible or unreasonable question. It is not there to trick you or catch you out, it is to test how you will react to something you will encounter in reality.
-
1\$\begingroup\$ As the question mentions
USE character array to perform this operation in place
it probably doesn't except the use ofreplaceAll
\$\endgroup\$D. Jurcau– D. Jurcau2017年03月03日 20:22:25 +00:00Commented Mar 3, 2017 at 20:22 -
1\$\begingroup\$ The internal implementation of a Java String is a char array. However that is bogus requirement anyway, they do not and should not dictate implementation. Just because a blog has re-printed a facsimile of an interview question doesn't mean the person that posted it understand the marking or the expectations of the interviewer. I would treat that as a trap to sort the wheat from the chaff. I've set out my criteria and I would be confident saying others would take a similar view on re-using and not inventing the wheel. \$\endgroup\$Martin Spamer– Martin Spamer2017年03月03日 21:00:51 +00:00Commented Mar 3, 2017 at 21:00
How come nobody notices the name of the method conversion()
? Shouldn't your method names be verbs? convert()
is a better method name in this case.
-
\$\begingroup\$ Good catch, someone mentioned this before.Thank you. \$\endgroup\$TheLearner– TheLearner2017年03月04日 23:45:37 +00:00Commented Mar 4, 2017 at 23:45
-
\$\begingroup\$ I'd call it
urlEncodeSpaces()
orpercentEncodeSpaces()
. \$\endgroup\$Gerold Broser– Gerold Broser2018年11月23日 22:53:40 +00:00Commented Nov 23, 2018 at 22:53
You ignored the instructions
According to the problem statement, the String
passed in has enough room to hold all the expanded characters, and you are supposed to do the expansion in-place. You went ahead and allocated a second character array and copied the characters from one array to another.
The whole backwards loop you used only makes sense if you are expanding in-place in the same array. If you allocate a new array, you might as well write the loop in the forwards direction since it doesn't make any difference.
Other things
- I find it easier to read an if-else statement if the if condition is written in the positive sense
if (c == ' ')
rather thanif (c != ' ')
. - Although your backwards loop does the right thing, I had to stare at it for a long time to convince myself that it was correct. Instead of an expression for the insertion point, I would just use another index.
(I just noticed that @rolfl already covered these points)
Rewrite
Here is how I would have modified your function. (It looks a lot like @rolfl's version actually):
public static String conversion(String str, int length)
{
char[] strChars = str.toCharArray();
int numSpaces = 0;
for (int i = 0; i < length; i++) {
if (strChars[i] == ' ') {
numSpaces++;
}
}
int newLength = length + 2 * numSpaces;
for (int i = length - 1, j = newLength - 1; i >= 0; i--) {
char c = strChars[i];
if (c == ' ') {
strChars[j--] = '0';
strChars[j--] = '2';
strChars[j--] = '%';
} else {
strChars[j--] = c;
}
}
return String.valueOf(strChars);
}
A tad bit on the concerned side here as I am not sure if your goal is a brute force coding requirement or not but the above link shows how to use the string split function an example is
String[] out = string.split("-");
Then just reassemble the string from out[]
-
\$\begingroup\$ »USE character array to perform this operation in place« was one of the requirements. And I would not call that in place \$\endgroup\$Thomas Junk– Thomas Junk2017年03月03日 19:14:25 +00:00Commented Mar 3, 2017 at 19:14
Explore related questions
See similar questions with these tags.