HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Setting output width with ostream_iterator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withoutputostream_iteratorsettingwidth

Problem

I'd like to display a vector (implemented in my custom Vector class) and have the output width of the elements be configurable. I've done this by creating a simple wrapper for the element being displayed which stores the desired width in a static field. I'm not sure if this is the best approach.

/** A struct for formatting doubles with a fixed width. */
struct FixedWidthDouble {
  FixedWidthDouble(double x) : x_(x) {}
  double x_;
  static int width_;
};

/** Apply a fixed width manipulator to a double for display. */
std::ostream& operator<<(std::ostream& out,
    const FixedWidthDouble &fixedWidthDouble) {
  return out << std::setw(fixedWidthDouble.width_) << fixedWidthDouble.x_;
}


Here's an example of it being applied:

/** Convert a Vector into a stream suitable for display. */
std::ostream& Vector::Print(std::ostream& out, int width) const {
  out (out, " "));
  return out << " ]";
}


Is there a cleaner way to do this?

Proposed Solution

Based on feedback from Begemoth this is the solution I decided to go with. Does this look reasonable?

template
struct FixedWidthDouble {
  FixedWidthDouble(double x) : x_(x) {}
  double x_;
};

/** Apply a fixed width manipulator to a double for display. */
template 
std::ostream& operator &fixedWidthDouble) {
  return out 
std::ostream& Vector::Print(std::ostream& out) const {
  out  >(out, " "));
  return out << " ]";
}

Solution

There are one problem with your approach you make local information global as consequence your FixedWidthDouble class is not thread-safe. There is a simple way to solve the problem by turning width_ into a integral template argument of the class template template FixedWidthDouble.

In C++11 I'd use range-based for:

std::ostream& Vector::Print(std::ostream& out, int width) const
{
  out << "[" << std::setprecision(1) << std::fixed;
  for (auto& x : *this)
    out << std::setw(width) << x;
  return out << " ]";
}


In the old C++98 it is cleaner to use old for with iterators. Also you can create a new iterator type that will set the width:

template >
class width_ostream_iterator
  : public std::iterator
{
public:
  typedef CharT char_type;
  typedef Traits traits_type;
  typedef std::basic_ostream ostream_type;

private:
  ostream_type* stream;
  const CharT* separator;
  int width;

public:
  explicit width_ostream_iterator(ostream_type& stm, int width, const CharT* separator = 0)
    : stream(&stm), separator(separator), width(width)
  { }

  width_ostream_iterator& operator=(const T& value)
  {
    *stream << std::setw(width) << value;
    if (separator) *stream << separator;
    return *this;
  }

  width_ostream_iterator& operator*()     { return *this; }
  width_ostream_iterator& operator++()    { return *this; }
  width_ostream_iterator& operator++(int) { return *this; }
};


Update: Your proposed solution is valid if the width can only be changed at compile time, if you want to change the width at the runtime (e.g. from configuration file or command line arguments) you have to use plain for loop or the width_ostream_iterator I've provided. One thing I do not like about the width_ostream_iterator — it is definitely a library entity intended to be used in multiple places but it allows you only to change the width of the field and nothing else. It is too specific for the library (it does not allow an arbitrary manipulator to a stream before outputting a value) it is too complicated for a single use.

And one more note: the Print function uses only public interface of the vector (I assume that begin and end functions are public), so it does not required to be a member function. Prefer to make such functions free not member. Why not use member function for all class operations? You can extend the interface of a class without modifying it, e.g. you can have a Vector class and stream I/O operations defined in separate headers, you can later add another I/O operations (e.g. for QDataStream) in a third header and you will not have to recompile the parts of the program that depend only on the first and second headers. Your Print function can potentially work with any class which looks and behaves like a standard sequence, just like any algorithm from the standard library. The class (just a function) must do one thing well, the Vector class stores a number of values of some type and provides constant-time random-access to the elements, it doesn't sort elements (that what std::sort algorithm is for), it does not search for elements (that what std::find and std::find_if algorithms are for), it does nit provide elementwise arithmetic operations (oops, there is no std::zip to combine several sequences into one), etc.

Code Snippets

std::ostream& Vector::Print(std::ostream& out, int width) const
{
  out << "[" << std::setprecision(1) << std::fixed;
  for (auto& x : *this)
    out << std::setw(width) << x;
  return out << " ]";
}
template<typename T, typename CharT = char, typename Traits = std::char_traits<CharT> >
class width_ostream_iterator
  : public std::iterator<std::output_iterator_tag, void, void, void, void>
{
public:
  typedef CharT char_type;
  typedef Traits traits_type;
  typedef std::basic_ostream<CharT, Traits> ostream_type;

private:
  ostream_type* stream;
  const CharT* separator;
  int width;

public:
  explicit width_ostream_iterator(ostream_type& stm, int width, const CharT* separator = 0)
    : stream(&stm), separator(separator), width(width)
  { }

  width_ostream_iterator& operator=(const T& value)
  {
    *stream << std::setw(width) << value;
    if (separator) *stream << separator;
    return *this;
  }

  width_ostream_iterator& operator*()     { return *this; }
  width_ostream_iterator& operator++()    { return *this; }
  width_ostream_iterator& operator++(int) { return *this; }
};

Context

StackExchange Code Review Q#18291, answer score: 6

Revisions (0)

No revisions yet.