nus-cs2103-AY1819S1 / forum

A repo to hold the issue tracker for module discussions
1 stars 1 forks source link

1718Sem1 Q4 attempted answer #213

Open davidchoo12 opened 5 years ago

davidchoo12 commented 5 years ago

image

  1. using guard clause if (original.isEmpty()) at the start to return "", then the rest of the code does not need to be wrapped in else block
  2. the loop should just do getNumber(original.charAt(i)) and concatenate it to a new string variable, finally returning the new variable instead
  3. log message can include the argument input
  4. header comment "remove" should be "removes"
  5. method name can be clearer, like "convertFirstTenCharToIntString"

The last 2 are quite nitpicky, I wonder if there is a better answer?

Also, how is the STATUS_UNSTABLE variable an example suggestion? Why are the 3 static variables there in the first place? image

CrimsonAng commented 5 years ago

If i not wrong your 4. should be ok as [Write the first sentence as a short summary of the method, as Javadoc automatically places it in the method summary table (and index). In method header comments, the first sentence should start in the form Returns ..., Sends ..., Adds ... (not Return or Returnning etc.)]

if I not wrong, the suggestion is making all constants having the same preamble if they are the same grouping,

so the third statement is [private static int STATUS_UNKNOWN = 2;]

1) maybe the logger info could change to [logger.info("Attempting the encode method");] as the word entering may suggest a change will occur? but not all cases the string may be encoded

2) Using a new variable instead of constantly reusing the [original] variable? so the final return is not the original string but actually encoded string. but it doesn't seemed to improve more readability e.g [String encodedString = original encodedString = removeSpaces(encodedString); encodedString = removeNumbers(encodedString); ... return encodedString;]

damithc commented 5 years ago

The given answers by both of you are acceptable ones. Example suggestion is just a dummy to show how to give a suggestion.

pangjiahao commented 5 years ago

Would the loop be a violation of the SLAP principle?

damithc commented 5 years ago

Would the loop be a violation of the SLAP principle?

Yes, we can accept that as a valid answer too.

jjlee050 commented 5 years ago

Is it possible to provide more than 5 code quality improvements? Would there be any marks deducted for invalid improvements despite having more than 5?

damithc commented 5 years ago

Is it possible to provide more than 5 code quality improvements? Would there be any marks deducted for invalid improvements despite having more than 5?

Yes you can give more than 5 in which case I will ignore low-value suggestions in favor of higher value suggestions. But wrong suggestions will count against you.

jjlee050 commented 5 years ago

Noted. Is adding space between the logging statement and before the if statement in order to increase readability by provide different blocks of codes acceptable as improvement?

damithc commented 5 years ago

Is adding space between the logging statement and before the if statement in order to increase readability by provide different blocks of codes acceptable as improvement?

Yes.

jjlee050 commented 5 years ago

Thanks for the clarification