# Simple stringifying using << operator

Background

I wrote my_length class and some code. my_length can be stringified using << operator, and main() tests this.

Questions

1. I think operator<< can be refactored. How can I rewrite operator<< of my_length, and make it better code?
2. How can I make rest of code better?

Code

#include <vector>
#include <string>
#include <cstdint>
#include <iostream>

class my_length {
int id;
std::string name;
uint64_t v;     // unit is 'mm'
bool fmt_auto;

public:
my_length (int id, std::string name, uint64_t v)
: id{id}, name{name}, v{v}, fmt_auto(false) {}
friend std::ostream &operator<<(std::ostream &out, const my_length &l) {
out << l.id << "/" << l.name << " : ";
if (!l.fmt_auto) {
return out << l.v << "mm";
} else {
if (l.v<10)
return out << l.v << "mm";
else if (l.v<1000)
return out << double(l.v)/10 << "cm";
else if (l.v<1000000)
return out << double(l.v)/1000 << "m";
else
return out << double(l.v)/1000000 << "km";
}
}
void set_fmt_auto(bool b) {fmt_auto = b;}
};

bool need_fmt_auto(int x) {
return (x%2) != 0;
}

int main() {
std::vector<my_length> lengths;
my_length l{0, "a", 1};
lengths.push_back(l);
uint64_t val = 17;
for (int i=1; i<10; i++) {
my_length l{i, std::string(1, 'a'+i), val};
lengths.push_back(l);
val *= 10;
}
for (int i=0; i<lengths.size(); i++) {
if (need_fmt_auto(i)) {
lengths[i].set_fmt_auto(true);
std::cout << "fmt_auto: ";
}
std::cout << lengths[i] << "\n";
}
}

• What's the purpose of this class? Once it's initialized you can't even access v, just switch fmt_auto. In which context would one want to create an instance? Why not just use a function std::string length_with_unit(uint64_t v, bool fmt_auto=false) or something similar? – Yurim Dec 12 '18 at 7:06
• I updated my code to clarify the purpose of my_length. – H. Jang Dec 12 '18 at 8:13
• @Yurim Maybe std::string length_with_unit() is a good choice. But I think this function can be used only with my_length. And << operator does not create string instance unlike length_with_unit(), so it makes little advantage on performance. Because of this two reasons, I write this as << operator. – H. Jang Dec 12 '18 at 8:20
• I still fail to imagine a use case where I would want to create an instance of my_length because all I can do with it is to write it to an std::ostream. Why would I want to write my_length l(42,name,567); l.set_fmt_auto(true); std::cout << l; instead of std::cout << length_with_unit(42, name, 567, true); or even std::cout << 42 << '/' << name << " : " << length_with_unit(567, true);? What's your high-level motivation to write this class? – Yurim Dec 12 '18 at 8:33
• @Yurim It is just a toy program. I wrote this code to study c++. I just wondered "my << operator is good enough" – H. Jang Dec 13 '18 at 8:09

## Gather constants together

The relationship among the numerical values that determine which unit is used and the divisor and the actual unit name is contained within a single function, which is good, but it could be more clear if expressed as a structure. For example, one could use this:

struct Units {
int limit;
double divisor;
std::string name;
};
static const std::array<Units,4> units;


Then outside of the class, we supply the values:

const std::array<my_length::Units,4> my_length::units = {{
{ 10, 1, "mm" },
{ 1000, 10, "cm" },
{ 1000000, 1000, "m" },
{ 0, 1000000, "km" },
}};


This brings things into much closer proximity so that it can be more easily seen and understood. We can do a little better with C++17 by using std::string_view instead of std::string for the unit name.

## Isolate concerns

It's usually better to have each function just do one thing. So insted of deciding on units and also printing, I'd suggest splitting those into two, using a private member function:

const Units& selectUnit() const {
auto ret{units.cbegin()};
if (fmt_auto) {
while (ret->limit && v >= ret->limit) {
++ret;
}
}
return *ret;
}


Now the operator<< looks like this:

friend std::ostream &operator<<(std::ostream &out, const my_length &l) {
const Units u{l.selectUnit()};
return out << l.id << "/" << l.name << " : "
<< l.v/u.divisor << u.name;
}


## Use standard algorithms

The test code starts with this:

std::vector<my_length> lengths;
my_length l{0, "a", 1};
lengths.push_back(l);
uint64_t val = 17;
for (int i=1; i<10; i++) {
my_length l{i, std::string(1, 'a'+i), val};
lengths.push_back(l);
val *= 10;
}


I'd suggest that one could use std::iota instead. I'll leave it to you to create the details of that.

## Reconsider the design

Once constructed, there is no capability to do anything with the my_length` object except to print it. If all that's needed is printing, then it may be better to use a freestanding function in a namespace instead of an object, as suggested in the comments. If not, then it makes sense to provide those other operations within the class.

• You beat me to the punch on that one. The only difference I would have made is rather than the units be decided by the class get the user to define the units by allowing a manipulator that is applied first. – Martin York Dec 13 '18 at 20:24
• You are right; that would be an improvement. – Edward Dec 14 '18 at 0:31