Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

That entire code could be reduced to these few lines:

private String timeDescription(String description, int seconds) {
 return putTimeInXX(description, secondsToString(seconds));
}
private String secondsToString(int seconds) {
 return String.format("%02d:%02d", seconds / 60, seconds % 60);
}
private String putTimeInXX(String description, String timeString) {
 return description.replaceAll("XX", timeString);
}

@rolfl @rolfl already pointed out about secondsToString.

The int pTime parameter was not well-named: the Hungarian notation is not recommended, simple English words are better. In this case, the method formats seconds to string, so seconds would be a much more natural name. I changed all the variable names following this logic.

The putTimeInXX method was too tedious: splitting the input string on XX just to replace it? It's a lot easier and natural to use String.replace or String.replaceAll. Also, I would recommend using %s in the input string, that way you could replace the %s template even more efficiently using String.format:

private String putTimeInXX(String description, String timeString) {
 return String.format(description, timeString);
}

In the timeDescription method, the intermediary final variables were really pointless. You can simply insert the method calls in a single expression, and the line is still not too long, and you have a much shorter method.

That entire code could be reduced to these few lines:

private String timeDescription(String description, int seconds) {
 return putTimeInXX(description, secondsToString(seconds));
}
private String secondsToString(int seconds) {
 return String.format("%02d:%02d", seconds / 60, seconds % 60);
}
private String putTimeInXX(String description, String timeString) {
 return description.replaceAll("XX", timeString);
}

@rolfl already pointed out about secondsToString.

The int pTime parameter was not well-named: the Hungarian notation is not recommended, simple English words are better. In this case, the method formats seconds to string, so seconds would be a much more natural name. I changed all the variable names following this logic.

The putTimeInXX method was too tedious: splitting the input string on XX just to replace it? It's a lot easier and natural to use String.replace or String.replaceAll. Also, I would recommend using %s in the input string, that way you could replace the %s template even more efficiently using String.format:

private String putTimeInXX(String description, String timeString) {
 return String.format(description, timeString);
}

In the timeDescription method, the intermediary final variables were really pointless. You can simply insert the method calls in a single expression, and the line is still not too long, and you have a much shorter method.

That entire code could be reduced to these few lines:

private String timeDescription(String description, int seconds) {
 return putTimeInXX(description, secondsToString(seconds));
}
private String secondsToString(int seconds) {
 return String.format("%02d:%02d", seconds / 60, seconds % 60);
}
private String putTimeInXX(String description, String timeString) {
 return description.replaceAll("XX", timeString);
}

@rolfl already pointed out about secondsToString.

The int pTime parameter was not well-named: the Hungarian notation is not recommended, simple English words are better. In this case, the method formats seconds to string, so seconds would be a much more natural name. I changed all the variable names following this logic.

The putTimeInXX method was too tedious: splitting the input string on XX just to replace it? It's a lot easier and natural to use String.replace or String.replaceAll. Also, I would recommend using %s in the input string, that way you could replace the %s template even more efficiently using String.format:

private String putTimeInXX(String description, String timeString) {
 return String.format(description, timeString);
}

In the timeDescription method, the intermediary final variables were really pointless. You can simply insert the method calls in a single expression, and the line is still not too long, and you have a much shorter method.

added 16 characters in body
Source Link
janos
  • 113k
  • 15
  • 154
  • 396

That entire code could be reduced to these few lines:

private String timeDescription(String description, int seconds) {
 return putTimeInXX(description, secondsToString(seconds));
}
private String secondsToString(int seconds) {
 return String.format("%02d:%02d", seconds / 60, seconds % 60);
}
private String putTimeInXX(String description, String timeString) {
 return description.replaceAll("XX", timeString);
}

@rolfl already pointed out about secondsToString.

The int pTime parameter was not well-named: the Hungarian notation is not recommended, simple English words are better. In this case, the method formats seconds to string, so seconds iswould be a much more natural name. I changed all the variable names following this logic.

The putTimeInXX method was too tedious: splitting the input string on XX just to replace it? It's a lot easier and natural to use String.replace or String.replaceAll. Also, I would recommend using %s in the input string, that way you could replace the %s template even more efficiently using String.format:

private String putTimeInXX(String description, String timeString) {
 return String.format(description, timeString);
}

In the timeDescription method, the intermediary final variables were really pointless. You can simply insert the method calls in a single expression, and the line is still not too long, and you have a much shorter method.

That entire code could be reduced to these few lines:

private String timeDescription(String description, int seconds) {
 return putTimeInXX(description, secondsToString(seconds));
}
private String secondsToString(int seconds) {
 return String.format("%02d:%02d", seconds / 60, seconds % 60);
}
private String putTimeInXX(String description, String timeString) {
 return description.replaceAll("XX", timeString);
}

@rolfl already pointed out about secondsToString.

The int pTime parameter was not well-named: the Hungarian notation is not recommended, simple English words are better. In this case, the method formats seconds to string, so seconds is a natural name. I changed all the variable names following this logic.

The putTimeInXX method was too tedious: splitting the input string on XX just to replace it? It's a lot easier and natural to use String.replace or String.replaceAll. Also, I would recommend using %s in the input string, that way you could replace the %s template even more efficiently using String.format:

private String putTimeInXX(String description, String timeString) {
 return String.format(description, timeString);
}

In the timeDescription method, the intermediary final variables were really pointless. You can simply insert the method calls in a single expression, and the line is still not too long, and you have a much shorter method.

That entire code could be reduced to these few lines:

private String timeDescription(String description, int seconds) {
 return putTimeInXX(description, secondsToString(seconds));
}
private String secondsToString(int seconds) {
 return String.format("%02d:%02d", seconds / 60, seconds % 60);
}
private String putTimeInXX(String description, String timeString) {
 return description.replaceAll("XX", timeString);
}

@rolfl already pointed out about secondsToString.

The int pTime parameter was not well-named: the Hungarian notation is not recommended, simple English words are better. In this case, the method formats seconds to string, so seconds would be a much more natural name. I changed all the variable names following this logic.

The putTimeInXX method was too tedious: splitting the input string on XX just to replace it? It's a lot easier and natural to use String.replace or String.replaceAll. Also, I would recommend using %s in the input string, that way you could replace the %s template even more efficiently using String.format:

private String putTimeInXX(String description, String timeString) {
 return String.format(description, timeString);
}

In the timeDescription method, the intermediary final variables were really pointless. You can simply insert the method calls in a single expression, and the line is still not too long, and you have a much shorter method.

Source Link
janos
  • 113k
  • 15
  • 154
  • 396

That entire code could be reduced to these few lines:

private String timeDescription(String description, int seconds) {
 return putTimeInXX(description, secondsToString(seconds));
}
private String secondsToString(int seconds) {
 return String.format("%02d:%02d", seconds / 60, seconds % 60);
}
private String putTimeInXX(String description, String timeString) {
 return description.replaceAll("XX", timeString);
}

@rolfl already pointed out about secondsToString.

The int pTime parameter was not well-named: the Hungarian notation is not recommended, simple English words are better. In this case, the method formats seconds to string, so seconds is a natural name. I changed all the variable names following this logic.

The putTimeInXX method was too tedious: splitting the input string on XX just to replace it? It's a lot easier and natural to use String.replace or String.replaceAll. Also, I would recommend using %s in the input string, that way you could replace the %s template even more efficiently using String.format:

private String putTimeInXX(String description, String timeString) {
 return String.format(description, timeString);
}

In the timeDescription method, the intermediary final variables were really pointless. You can simply insert the method calls in a single expression, and the line is still not too long, and you have a much shorter method.

lang-java

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