3

I want to refactor some printf/sprintf/fprintf statements into ostream/sstream/fstream statements. The code in question pretty-prints a series of integers and floating-point numbers, using whitespace padding and fixed numbers of decimal points.

It seems to me that this would be a good candidate for a Martin Fowler style writeup of a safe, step-by-step refactorings, with important gotchas noted. The first step, of course, is to get the legacy code into a test harness, which I have done.

What slow and careful steps can I take to perform this refactoring?

Keith Pinson
  • 7,835
  • 7
  • 61
  • 104
  • Even if no one else cares to answer, I intend to use this post as a place to record my steps, to help others/myself in the future. – Keith Pinson Feb 05 '13 at 20:53
  • 1
    Why don't you use a formatting library that provides similar interface as `printf` but is type safe, like tinyformat (https://github.com/c42f/tinyformat) ? – vitaut Feb 05 '13 at 20:55
  • 1
    I don't think this is a good candidate for slow and careful refactoring: a shotgun refactoring would do just fine, because you are simply replacing calls of one part of the standard C++ library with calls of the other part. You are not refactoring the structure of your program, only its implementation detail, so as long as you have a good test suite, fire away and make as few or as many changes at a time. – Sergey Kalinichenko Feb 05 '13 at 20:58

2 Answers2

2

If refactoring is not the goal in itself, you can avoid it altogether (well, almost) by using a formatting library such as tinyformat which provides an interface similar to printf but is type safe and uses IOStreams internally.

vitaut
  • 49,672
  • 25
  • 199
  • 336
  • Because of the environment I work it, that is not an option. Fair answer, though, and good to know, so +1. – Keith Pinson Feb 05 '13 at 21:03
  • @Kazark: I am just curious, why exactly it is not an option? Is it prohibited by the company policy or something? – vitaut Feb 05 '13 at 21:06
  • Because of the politics/security constraints of the place, it is *very* difficult to get new software packages installed. :( – Keith Pinson Feb 05 '13 at 21:17
  • 2
    Boost also provides a print function, and is well-known enough that it might be usable. – ssube Feb 05 '13 at 21:38
  • @Karzak: So your organizational politics makes it easier to write and maintain custom code, rather than accept a well tested package? – Ira Baxter Feb 05 '13 at 21:46
  • I'm accepting my own answer because it more directly answers the question. However, your answer is probably the better one to go with in the long run for anyone who has the option. – Keith Pinson Feb 08 '13 at 15:21
  • @Ira Well, not because of our software team, but yes. There are real and reasonable security constraints...but also the IT team... well, this isn't really the place to talk about it, haha... – Keith Pinson Feb 08 '13 at 15:24
2

Basic mechanics of the conversion:

  • Convert each printf-style clause %w.pf or %w.pe, where w is the field width and p is the number of digits of precision, into << setw(w) << setprecision(p) << fixed.
  • Convert each printf-style clause %wd or %wi, where w is the field width, into << setw(w).
  • Convert "\n" to endl where appropriate.

Process for printf:

  • Create a char[] (let's call it text) with enough total width.
  • Convert the printf(...) to sprintf(text, ...), and use cout << text to actually print the text.
  • Complete using the common instructions.

Process for fprintf:

  • Same as printf, but use the appropriate fstream instead of cout.
    • If you already have an opened C-style FILE object that you do not want to refactor at this time, it gets a little sticky (but can be done).
  • Complete using the common instructions.

Process for sprintf:

  • If the string being written to is only used to output to a stream in the current context, refer to one of the two refactorings above.
    • Otherwise, begin by creating a stringstream and streaming the contents of the char[] you are writing to into that. If you are still intending to extract a char* from it, you can do std::stringstream::str().c_str().
  • Complete using the common instructions.

Common instructions:

  • Convert each clause one by one into C++-style.
  • Remove *printf and char[] declarations as necessary when finished.
  • Apply other refactorings, particularly "Extract Method" (Fowler, Refactoring) as necessary.
Community
  • 1
  • 1
Keith Pinson
  • 7,835
  • 7
  • 61
  • 104
  • 1
    Given that this transformation is this mechanical, you should consider a script to to do the job. Its pretty likely you can recognize the print calls with very low error rates, and the replacement is likely to be 100% right if done right. "Slow and incremental" might be the wrong approach. – Ira Baxter Feb 08 '13 at 16:29
  • You can leave `"\n"` as `"\n"`. `endl` is more like `"\n"` followed by `fflush`. – Nemo Feb 14 '13 at 17:15