nus-cs2103-AY2425S1 / forum

11 stars 0 forks source link

Practice test Part 1 Question [p1.12] implementation: refactoring #885

Closed annabellekk closed 2 days ago

annabellekk commented 4 days ago

Hello, may I ask for practice test part 1 question 12,

  1. Per the definition of refactoring from textbook, it says 'without modifying the external behaviour'. In this case why is option 4 not a refactoring? The answer says because it changes the behaviour, but I thought that adding an exception does not change the external behaviour (i.e. my understanding is the behaviour displayed to the user), but it does change the internal behaviour of the code since it now catches a new exception?

  2. Why is 5 a valid refactoring? My rationale in choosing option 5 was because it violates coding standard by removing the braces around the if block and thus is not "improving the internal structure" as per the definition of refactoring.

Screenshot 2024-11-23 at 9 48 20 PM Screenshot 2024-11-23 at 9 48 54 PM
emmannyyy commented 4 days ago

For 1: I think that the reason why 4 is not a valid option is because refactoring is not the same as bug fixing. image

As mentioned, the sort() method was not working, and the addition of an exception should fall under the definition of 'bug fixing'. In addition, it does indeed modify external behaviour. When an exception is thrown it is likely that the user will be displayed an error message, and 'gracefully' recover, as compared to crashing.

For 2 : While it does not fall under the list of common refactoring, (seen below), I believe it is still considered refactoring since it does improve the code, and makes it more readable. image

annabellekk commented 4 days ago

@emmannyyy Thanks for your answer! Part 1 makes sense now.

But for part 2, I don't quite get how removing the braces around the if block improves the code, since according to the coding standard, you should not remove the braces even if there is only one statement. Or is improving the code independent from the coding standard?

Screenshot 2024-11-24 at 8 37 24 AM
JagdeepSinghNUS commented 3 days ago

@emmannyyy I don't think removing brackets around an if statement would be considered improving the code to make it more readable. If anything it might make it worse in the context of Java.

I honestly had this same question as well. I think the real reason is less complicated than we think it is, I think the question was just trying to test us on which option changes the external behaviour. If you were to look for the model answer in A2 for that question, it just states, it changes the behaviour.

But with that being said you raised a good point, because by definition, Refactoring means: Improve internal structure without changing external behaviour. Technically, while removing brackets do not change the external behaviour, it is not explicitly: improving internal structure without changing external behaviour either. If anything it sounds like you edited your Java code by making it less readable. Not too sure if I myself may have misunderstood the concept or question.

annabellekk commented 3 days ago

@JagdeepSinghNUS Thanks for your input! Maybe Prof @damithc could clarify on this concept?

damithc commented 3 days ago

Good discussion, everyone.

  1. If the method doesn't work as expected for certain inputs, and now made to work for that input as well, it is a behaviour change, and hence, not refactoring.
  2. First, refactoring is a change done to the code in order to 'improve' its structure. Whether it really improves the code is a matter of opinion. Any code change will have pros and cons. So, even if we don't agree that the change has more pros than cons , it is still a refactoring. Second, going against the coding standard might be warranted in exceptional cases. Depends on the context. For example, if there are 10 consecutive if blocks each containing similar single statement, it may be better to omit braces for all of them, so that the series of if blocks takes less vertical space, and easier to read. Third, note that the question didn't say anything about following the coding standard that we used in our course. The coding standard Tom is following might not require (or even prohibit using) braces for single-line code blocks.
annabellekk commented 2 days ago

Got it, thank you!