

Refactoring: How do I even start? - thclark
http://www.socalledprogrammer.com/2015/03/refactoring-getting-started.html

======
a_bonobo
I can only recommend Feathers' Working Effectively With Legacy Code, which far
expands on OP's themes, notably OP is missing tests - how can you improve on
existing code when you don't even know that it works like it says it works?

In some cases it shows its age (especially when it comes to rolling your own
mock testing, most languages have automated frameworks for that now), but it's
still a great overview of the techniques you can employ.

------
michaelvkpdx
All professional programmers should continually read Martin Fowler's
"Refactoring", in the same way English professors read Shakespeare.

"Refactoring" does not mean "previous programmer was an idiot". Often, the
understanding of the problem and the way the code is used change over time.
Refactoring is a way of bringing the out-of-control weeds back into healthy
symbiosis with the larger garden of code.

I stopped telling clients a long time ago that I'm "refactoring" and instead I
just add some extra time to all tasks to account for refactoring on legacy
code.

------
zerohp
I know this code is just an example, but it seems like it's entirely
backwards. The "bad" code at the beginning is immediately clear in what it's
doing, but the refactored code is significantly longer and hard to understand.

------
crdoconnor
He missed step #1: write some tests that will provide feedback if you broke
something, or assurance that you didn't.

~~~
gedrap
This. The problem is that in some cases, the legacy system is really messed up
and making it unit testable is a big refactoring task on it's own.

In these cases, I try to write some functional tests first (such as calling
restful endpoints and checking the response, or using a headless browser). Not
great but much better than nothing. Any ideas on how to do it better?

~~~
blt
The closest thing I've seen to an actual definition of "legacy code" is "code
that's very difficult to unit test." It's a shitty catch-22.

~~~
philbarr
Feathers defines legacy code as code without accompanying tests in "Working
Effectively with Legacy Code". Even to the point where someone he knows said
of a particular shop, "they're writing legacy code, man!"

------
mbesto
Sandi Metz talk "All the Little Things"[0] is amazing for understanding the
general concepts around refactoring OOP code. It's ruby centric, but the
concepts are applicable to all OOP languages. The most interesting part is the
fact that during the refactoring process you should actually increase the
amount of code and complexity to eventually consolidate and aggregate it down.
This as opposed to simply trying to delete/mutate code.

[0] -
[https://www.youtube.com/watch?v=8bZh5LMaSmE](https://www.youtube.com/watch?v=8bZh5LMaSmE)

------
pmr_

        if( masterList[z].list2 != NULL && masterList[z].list2.length() > 0 )
        {
            for( Integer y = 0; y < masterList[z].list2.length(); y++ )
    

The if-statement is a good summary of what is wrong with Java. The author
doesn't even notice that the second argument of && is redundant and keeps it
in the "refactored" version as well...

~~~
Alupis
> The author doesn't even notice that the second argument of && is redundant
> and keeps it in the "refactored" version as well..

That's false. A list object may be initialized but be empty (making it's
length zero). Or it may not be initialized (making it null).

Both conditions may happen independently of one another. He checks for the
null first so that checking the length does not throw a NPE.

> The if-statement is a good summary of what is wrong with Java.

Frankly, I see nothing wrong here.

~~~
pmr_
He then proceeds to iterate over the list using zero based indexing. The loop
would not execute once if the list had length zero, making this check just
plain wrong.

I know an even lower level language (C++, C) that doesn't have the problem of
things which make no sense to be NULL (list elements). The problem is that
Java did away completely with value types and made everything pointer only.
That has been recognized by later languages (C#) and fixed.

The whole code consists of problems: the first is the language, the second is
the programmer, the third is the missing for-each construct.

~~~
Alupis
> I know an even lower level language (C++, C) that doesn't have the problem
> of things which make no sense to be NULL (list elements)

You must null check a lot of things in C, especially since there is no
graceful error handling (try/catch blocks)... C certainly allows things to be
null (or garbage) values.

> The problem is that Java did away completely with value types and made
> everything pointer only.

I'm not sure what you are saying here -- the very notion of pointers do not
exist in Java. This decision was made while creating the language, and avoids
an entire class of programming errors. Java is strictly pass by value.

> the third is the missing for-each construct.

I completely agree with you here. Java does have a for-each construct, and
it's recommended to use whenever possible. It avoids an entire class of
programming errors.

~~~
burntsushi
> I'm not sure what you are saying here -- the very notion of pointers do not
> exist in Java. This decision was made while creating the language, and
> avoids an entire class of programming errors. Java is strictly pass by
> value.

To clarify: the GP is probably referring to boxed and unboxed data types.
IIRC, Java has some unboxed data types ("primitive" types?), but mostly
everything is boxed behind a pointer.

~~~
Alupis
> but mostly everything is boxed behind a pointer.

Behind a Reference would be more accurate. It's just a reference to a spot in
the heap. Other than similarly "pointing" to a place in memory, the comparison
between Java References and C Pointers stops there. One cannot pass a
"pointer" in Java, nor can the pointer be free-form manipulated like in
pointer-arithmetic.

~~~
burntsushi
My statement is perfectly accurate in the context, which is discussing the
_representation_ of Java types.

Java doesn't hold a monopoly on the word "pointer." For example, Go has
pointers but doesn't allow pointer arithmetic in safe code. Similarly for
Rust.

------
limeyx
#1 If there are existing test cases \- Run them \- make sure they pass \- make
sure it looks like they have good coverage

#2 if there are no good test cases, write them and go to #1

------
pnathan
Worth noting: some shops will summarily reject on code review any refactor
that isn't part of the ticket/issue.

~~~
QuercusMax
Along the same lines: it's considered good practice to perform a refactoring
in a separate commit from code which changes behavior. This makes it much
easier to review the code, as you can look at the changes in isolation. If you
refactor and then change behavior in a single commit, it can result in a very
confusing change.

~~~
pnathan
Some shops reject commits that are refactoring. "Deliver value, don't just
goof off". :-)

~~~
QuercusMax
Sounds like they'd be awful places to work.

------
g8gggu89
Why does he keep using Integer instead of int? And why use Object everywhere
instead of generics?

------
OJFord
I'm so frustrated by the minimum width, that I'm commenting instead of
reading.

------
TeMPOraL
<<So-Called "Programmer">> is the name of the blog; the title of submitted
post is "Refactoring: How do I even start?". Could mods change this please?

~~~
pandatigox
That's a good point. I clicked on it thinking it was one of those depressing
rants about programming. Misleading and disappointed OP!

