Refactoring: Improving the Design of Existing Code

I think this is the sort of thing that enters the collective consciousness and then gets generalized, but code smells were originally pretty specific anti-patterns that had specific fixes. All from this book if I recall correctly

https://www.amazon.com/Refactoring-Improving-Design-Existing...

http://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672

Is the defacto standard for basic refactoring vocabulary everywhere I've worked.

book about refactoring: http://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672

The method that has worked best for me is to grab a copy from source control, with the intention of throwing this version away...

Then try and refactor the code. It is even better if you can refactor the code that you know you will be working on at a later stage.

The reason this is effective is because:

• refactoring gives you a goal for you to aim towards. Whereas "playing" an "breaking" the code is great - it is unfocused.
• To refactor code you really have to understand the code.
• Refactored code leaves code that has less concepts to retain in memory. If you don't understand a large codebase its not because you are a graduate - its because nobody can retain more than 7 (give or take a few) concepts at a time.
• If you follow correct refactoring guidelines it means you will be writing tests. Although, make sure that you will be working on the modules that you are testing as writing tests can be very time consumning (although very rewarding)

Do invest in buying this book at some point:

http://www.amazon.co.uk/Refactoring-Improving-Design-Existing-Technology/dp/0201485672

But these links should get you started:

Signs that your code needs refactoring and what refacoring to use (From Refactoring - Martin Fowler) http://industriallogic.com/papers/smellstorefactorings.pdf

A taxonomy of code smells: http://www.soberit.hut.fi/mmantyla/BadCodeSmellsTaxonomy.htm

Good luck!!!

Take a look in Martin Fowler's Refactoring: Improving the Design of Existing Code and Refactoring to Patterns by Joshua Kerievsky. These in turn reference the GoF Design Patterns book so get that too.

If you can beyond the basic Rename Feature and Extract Function then you might be onto a winner.

First, tell your boss you're not continuing until you have:

http://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672

and to a lesser extent:

http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052

Secondly, there is no way of modularising code by shoe-horning it into C++ class. This is a huge task and you need to communicate the complexity of refactoring highly proceedural code to your boss.

It boils down to making a small change (extract method, move method to class etc...) and then testing - there is no short cuts with this.

I do feel your pain though...

I have over 20 years of programming experience, and in my experience some good ways to improve your programming skills are (not in any order of priority)

a) Solve complex programming problems

b) Revisit your solutions to see where improvements can be made (2-3 passes at least).A good book with tips to improve your programs is refactoring: http://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672

c) Dr. Dobbs is an excellent site to get tips and insight: http://www.drdobbs.com/

e) Look at other people's code, eg. open source code so you don't develop a frog in the well mindset. This is a great way to learn good practices.

f) Learn to program in multiple languages (eg Java, PHP). This is also a great way to improve skills.

g) Try and always think 'best practices' when writing code. HTH.

There are some resources on the Anti-IF Campaign site, such as this article.

I believe it's a matter of degree. Conditionals aren't always bad, but they can be (and frequently are) abused.

Additional thoughts (one day later)

Refactoring: Improving the Design of Existing Code is a good reference on this subject (and many others). It covers Replace Conditional with Polymorphism. There's also a new one, Replace Conditional with Visitor, on the web site.

I value simplicity and single responsibility over removing all if statements. These three goals often coincide. Static analysis tools that support the cyclomatic complexity metric can quickly point out code with nested or serial conditionals. The if statements may remain post-refactoring, but could be broken out into smaller methods and/or multiple classes.

Update: Michael Feathers wrote an article on Unconditional Programming.

This is a popular topic: Phil Haack on Death to the IF statement!

I agree that this "special case" discussion, in the Define Normal Flow section of the Error Handling chapter of Martin's excellent Clean Code, is a bit unswifty.

As you point out, part of Martin's rationale is to excise the repeated use of Java's "awkward" (his words, not mine) try-catch pattern, especially if you're repeating it a lot. But in Swift, if there are no expenses, you would probably not pursue an error throwing pattern, but rather just return an optional. And, as you say, the Swift nil-coalescing pattern is comparably elegant to the try-catch pattern. (As an aside, Swift optionals renders irrelevant the next two sections in that chapter, too, Don't Return Null and Don't Pass Null.)

Robert Martin credits Martin Fowler in this discussion, but in Refactoring, Fowler discusses "special case" pattern in the context the challenges of null checks in Java and all the problems that entails. But Swift handles optionals so much more gracefully and it renders much of Fowler's defensive-null discussions moot.

So, I agree, that MealExpenses is not a particularly good candidate for the "special case" pattern in Swift. (Frankly, I'm not sure it's a great candidate in general, regardless.) Optionals would be a more natural Swift solution. Now, if I were littering my code with the identical nil coalescing pattern all over the place, I'd look for ways to avoid repeating it, but I wouldn't necessarily jump to introduce the "special case" pattern solely for this purpose.

It's worth noting that in Refactoring, Fowler offers an expanded "special case" example, where a landlord's "customer" for some random building might be "no customer" (e.g. the building is vacant), an "unknown customer" (you know that there is a tenant, but you don't know who it is), or a particular customer. This ternary state is one where we have to start to think beyond a simple Swift optional. But at that point we might use an enum with associated values, or you could conceivably use the "special case" pattern (but now needing to introduce a protocol). It depends upon particular situation. With all of that said, I've yet to encounter scenarios where I've felt inclined to add "special case" pattern in my Swift code.

One way is that you can refactor it using ternary operator, but at the cost of readability.

def cart
  if user_signed_in?
    @user = current_user
    @cart = @user.cart.present? ? @user.cart : false
  else
    cart_id = session[:cart_id]
    @cart = cart_id.present? ? Cart.find(cart_id.to_i) : false
  end
end

Secondly, if you are compelled to write a very long method, it means there is something wrong with your Object Oriented Design. Maybe, you need to design a new class for the extra functionality, or you need to split the functionality in the same class by writing multiple methods that when combined, do the job of a single long method.

Why is it bad to write a method with too many lines?

Just like an essay with big paragraphs is harder to read, likewise a program with longer methods is difficult to read, and less likely to re-usable. The more chunks you divide your code in, the more moduler, re-usable and understandable your code will be.

What if we have to do a lot of work in it?

If you have to do a lot of work in it; it surely means that you have designed your classes in a way that isn't good. Perhaps, you need to design a new class to handle this functionality, or you need to split up this method into smaller chunks.

How can I re-factor this and write good code?

For that, I highly recommend to you a great book named: Refactoring by Martin Fowler, he is incredibly amazing at explaining how, when, and why to refactor your code.

At first glance, you could use the "Extract Method" refactoring a couple of times.

This code repeats:

ZMsg msg = new ZMsg();
    msg.add(pendingMessage.getEncodedRecords());
    try {
      // this returns instantly
      return msg.send(liveSocket.get().getSocket());
    } finally {
      msg.destroy();
    }

so make something like private void sendMsg() out of it. This code also repeats

PendingMessage m = new PendingMessage(address, encodedRecords, false);
    cache.put(address, m);
    try {
      if (doSendAsync(m, socket)) {
        return m.waitForAck();
      }
      return false;
    } finally {
      cache.invalidate(address);
    }

so make another method out of it.

In general, there's a classic and excellent book on refactoring https://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672

Well .. step nr.1: read this book. You have time enough for that till September.

Watch following lectures:

• It Was Like That When I Got Here
• Recognising smelly code
• Unit testing

These materials should give you some sense of the subject.

As for real first steps, separating the HTML from JavaScript is a nice place to begin. If you know how to do event delegation in javascript - good. If not, look into it.

Then you can move on to fixing the part of page that spews out the HTML. Separate the SQL, add some abstraction, some OOP principles.

If you end up aiming at something MVC-shaped, then this list of links might help a bit.

for automated software unit tests I would recommend google test. There is a very good q&a on this platform, which you can find here.

Additionally, there is CPPUnitLite, which is developed by the author of "Working Effectively with Legacy Code", Michael Feathers.

I used AutoIt Scripts for testing a MFC application just a little bit, but it was not that easy to maintain them properly and build an effective logging system for failed tests.

However, the unit tests depend heavily on the architecture of your program and the structure of your class - especially the dependencies to other components / classes. So if you already have an existing MFC application, which was not built with unit tests in mind, you probably have to refactor a lot of things. Therefore, I would recommend the mentioned book. You can also use the classic "Refactoring" by Martin Fowler.

May be you need to use Extract Class. It is common methodology for solving Feature Envy code smell (these are both taken from Martin Fowler's Refactoring).

About the many methods in one class - I reacall that I've read somewhere that the JVM will optimize additionaly your code, will inline methods, etc.

You can buy a copy of Refactoring: Improving the Design of Existing Code from Martin Fowler, you'll find a lot of things you can do during your refactoring operation.

Plus you can use tools provided by your IDE and others code analyzers such as Findbugs or PMD to detect problems in your code.

---

Resources :

• www.refactoring.com
• wikipedia - List of tools for static code analysis in java

On the same topic :

• How do you refactor a large messy codebase?
• Code analyzers: PMD & FindBugs

By all means, create the variable. A single variable like that will never, ever be a performance bottleneck, never.

Always favor readability. Performance problems usually appear only in specifics parts of a system. When they arise, and not before measuring those (aka having numbers), you treat them case by case. Never forget: Premature optimization is the root of all evil.

In your specific case, you could go even further and create another variable, explaining the intent of the comparison:

String host = args[0].toLowerCase();
boolean isRunningUnderProduction = host.equals("server");
if (isRunningUnderProduction) {
    System.out.println("SERVER");
}

Much better than a comment. And explains the intent of the code within a glimpse.

---

A quote from Martin Fowler's Refactoring book:

The interesting thing about performance is that if you analyze most programs, you find that they waste most of their time in a small fraction of the code. If you optimize all the code equally, you end up with 90 percent of the optimizations wasted, because you are optimizing code that isn't run much. The time spent making the program fast, the time lost because of lack of clarity, is all wasted time.

In other words: if your programs are easier to read, they are easier to fix the slow parts, when the time comes (if it comes).

Is there anything I can do except refactor the code?

No. Refactor the code. No method should be that long. Ever. Write small methods!

Seriously: any IDE there is will guide you through the refactoring, but it needs to be done. You might also want to read Refactoring: Improving the Design of Existing Code for guidance.

I would recommended (as others have):

• Refactoring: Improving the Design of Existing Code - Martin Fowler
• Test Driven Development: By Example - Kent Beck
• Patterns of Enterprise Architecture - Fowler et al.

A good or bad design reveals itself by how well it accomodates unexpected requirements, so I would suggest keeping a stock of potential "game features" handy to inform your design reflexions. Since you're doing this as a learning project you can afford to go crazy.

Arkanoid is a very good choice for this, it offers so many options. Make different bricks score different amounts of points. Make some bricks change the score of other bricks when hit. Make some bricks require multiple hits. Give superpowers to the ball, paddle, or bricks. Vary these powers: one of them makes the ball keyboard-controllable, another makes it transparent, another reverses "gravity", and so on. Make bricks drop objects.

The goal is that when you make such a change, it impacts the minimum possible number of classes and methods. Get a feel for how your design must change to fit this criterion.

Use an IDE that has a Refactoring menu, in particular the move method refactoring. (If you haven't, read the book Refactoring.) Experiment with placing your various methods here and there. Notice what becomes hard to change when the method is placed "wrong", and what becomes easier when you place it elsewhere. Methods are placed right when objects take care of their own state; you can "tell" an object to do something, rather than "ask" it questions about its state and then make decisions based on its answers.

Let's assume that in your design each sprite is an object instance. (You could choose other strategies.) Generally, motion alters the state of a sprite, so the method that describes motion for a particular kind of sprite probably belongs on that sprite's class.

Collision detection is a sensitive part of the code, as it potentially involves checking all possible pairs of sprites. You'll want to distinguish checking for collisions and informing objects of collisions. Your ball object needs to alter its motion on colliding with the paddle, for instance. But the algorithm for detecting collisions in general won't belong on the ball class, since other pairs of objects may collide with consequences that matter to the game.

And so on...

I've heard we should be preferring Composition over Inheritance.

Generally good advice. But it doesn't mean you should replace them all.

If your subclasses need all of what's in the superclasses and each subclass adds small bits of function, you might be fine to stay with inheritance.

How would it apply to these kind of scenarios where you want to use inheritance to remove duplicated code? Should I refactor the code to use Composition? How?

You can also use composition to remove duplicated code.

The refactoring you're looking for is Replace Inheritance with Delegation. Googling this phrase may find you a few references with more detail than a sample diagram, but you really ought to get the book Refactoring.