Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

###You ignored the instructions###

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###

Other things

  1. I find it easier to read an if-else statement if the if condition is written in the positive sense if (c == ' ') rather than if (c != ' ').
  2. 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###

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);
}

###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###

  1. I find it easier to read an if-else statement if the if condition is written in the positive sense if (c == ' ') rather than if (c != ' ').
  2. 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);
}

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

  1. I find it easier to read an if-else statement if the if condition is written in the positive sense if (c == ' ') rather than if (c != ' ').
  2. 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);
}
Source Link
JS1
  • 28.8k
  • 3
  • 41
  • 83

###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###

  1. I find it easier to read an if-else statement if the if condition is written in the positive sense if (c == ' ') rather than if (c != ' ').
  2. 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);
}
lang-java

AltStyle によって変換されたページ (->オリジナル) /