Let's look at that definition with a different emphasis:
Refactoring (noun): a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing it's external observable behaviour.
Cosmetic changes such as renaming variables or eliminating the explicit use of the default variable where it will be used implicitly are not refactorings. They don't affect the structure of the code.
- Add use strict and warnings
- Okay. This is actually not a refactoring in itself, but it's the first part of the common perl big refactoring 'Make Everything Compile with use strict and warnings'.
The Addition of warnings or strict checking is definitely not a refactoring of any sort. Doing so changes the "external observable behaviour" of the code. I'm not saying it shouldn't be done. I agree that it should.
- Split uses $_ by default, so don't need to state it
- Doesn't alter behaviour. You could even consider this the latter half of Remove Parameter if you look at it a certain way. Increases clarity. Valid refactoring.
Split uses $_ whether you state it or not. It isn't at all like Remove Parameter because the parameter is used regardless of whether it do explicitly. It is not an "unused parameter." For an inexperienced Perl programmer, reducing it is likely to reduce clarity. For an experienced programmer it should be just as clear either way. In any case, it doesn't change the structure of the code and is not a refactoring.
- Rename $id to $id_to_fetch since it's more descriptive
- Doesn't alter behaviour. Increases clarity. Valid refactoring.
This does not alter the structure of the code. It is not a refactoring.
- Since we're only interested in the school and name we can just extract those from the split ....
- Doesn't alter behaviour. Does increase clarity. Valid refactoring.
Whether or not this is a refactoring at all is debatable. The one significant change resulting from this is that some unnecessary values may not be stored. The original did keep unnecessary values in the array when the split resulted in more than 2 elements. If the split always resulted in exactly 2 elements, I would argue that this would not be a refactoring but I don't know that to be the case and suspect it isn't.
- We can then simplify the name return to 'return $name if $id eq $fetch_id;'
- Doesn't alter behaviour. For me, this does increase clarity. Valid refactoring.
Besides the fact that this isn't exactly "simplified" from the original return $schoolname;, it doesn't really alter the structure of the code.
Although you didn't point it out yourself, you did drop the unnecessary temporary variable, $schoolname, and I suppose that is a minor refactoring.
On the filehandle issue, localizing a typeglob and relying on perl to close your filehandle for you is not, IMHO, good practice. Keep in mind that a localized typeglob localizes all the variables with that name, not just the filehandle. Consider:
To get around that, you would need to use IO::Handle or allow perl to autovivify the handle for you (but only in 5.6.1 and above.) Doing so is often worth it but not for the sole purpose of obtaining a filehandle you don't have to explicitly close. In fact, closing the filehandle explicitly is generally considered a good practice.perl -le '$A = "a"; {local *A; $A = "b" } print $A'
Here's another quote from Refactoring: Improving the Design of Existing Code, this from the foreword:
So, what's the problem? Simply this: Refactoring is risky. It requires changes to working code that can introduce subtle bugs. Refactoring, if not done properly, can set you back days, even weeks. And refactoring becomes riskier when practiced informally or ad hoc. You start digging in the code. Soon you discover new opportunities for change, and you dig deeper. The more you dig, the more stuff you turn up. . .and the more changes you make. Eventually you dig yourself into a hole you can't get out of. To avoid digging your own grave, refactoring must be done systematically.Refactoring is risky. It shouldn't be done informally. It requires a disciplined systematic approach. Here at the monastery I've noticed that the term "refactoring" is both overused and often incorrectly used altogether. Refactoring is not the same as editing or rewriting and we shouldn't further the impression that is.
Refactoring is the practice of restructuring a program by changing its basic components. It is worthwhile to remember the mathematical roots of the word. One might accurately construct the number 24 with the "program" 2**2 * 3 + 6 + 2 * 3 * 1 but refactoring that to 2**3 * 3 results in a program which is shorter, clearer, and more efficient.
-sauoq "My two cents aren't worth a dime.";
In reply to What is refactoring? (was: Refactoring gone wrong)
by sauoq
in thread Cannot read in multiple lines
by Angel
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |