patterncppMinorCanonical
Integer to roman number converter
Viewed 0 times
romannumberintegerconverter
Problem
This is (yet another) integer to Roman numeral converter.
To give it a slightly interesting twist, I've implemented this as a
Here's the header:
```
#ifndef ROMAN_H_INC_
#define ROMAN_H_INC_
#include
#include
#include
#include
template >
class roman : public std::num_put {
public:
typedef charT char_type;
typedef OutputIterator iter_type;
protected:
virtual iter_type do_put(iter_type out, std::ios_base &, char_type, long v) const {
return fmt(v, out);
}
virtual iter_type do_put(iter_type out, std::ios_base &, char_type, long long v) const {
return fmt(v, out);
}
virtual iter_type do_put(iter_type out, std::ios_base &, char_type, unsigned long v) const {
return fmt(v, out);
}
virtual iter_type do_put(iter_type out, std::ios_base &, char_type, unsigned long long v) const {
return fmt(v, out);
}
std::string fmt_internal(unsigned long long value) const {
struct conv {
int val;
std::string rep;
};
static const conv table[] = {
{ 1000, "M" },
{ 900, "CM" },
{ 500, "D" },
{ 400, "CD" },
{ 100, "C" },
{ 90, "XC" },
{ 50, "L" },
{ 40, "XL" },
{ 10, "X" },
{ 9, "IX" },
{ 5, "V" },
{ 4, "IV" },
{ 1, "I" }
};
std::string roman;
for (auto const &c : table) {
while (value >= c.val) {
roman += c.rep;
value -= c.val;
}
}
return roman;
}
template
Iter fmt(T t, Iter i) const {
To give it a slightly interesting twist, I've implemented this as a
num_put facet of a C++ locale, so by creating the right locale, and imbuing a stream to use that locale, all the numbers written to that stream will be written out in Roman numerals (yeah, I know that's probably rarely useful or desirable, but it seemed like an interesting thing to do at the time).Here's the header:
```
#ifndef ROMAN_H_INC_
#define ROMAN_H_INC_
#include
#include
#include
#include
template >
class roman : public std::num_put {
public:
typedef charT char_type;
typedef OutputIterator iter_type;
protected:
virtual iter_type do_put(iter_type out, std::ios_base &, char_type, long v) const {
return fmt(v, out);
}
virtual iter_type do_put(iter_type out, std::ios_base &, char_type, long long v) const {
return fmt(v, out);
}
virtual iter_type do_put(iter_type out, std::ios_base &, char_type, unsigned long v) const {
return fmt(v, out);
}
virtual iter_type do_put(iter_type out, std::ios_base &, char_type, unsigned long long v) const {
return fmt(v, out);
}
std::string fmt_internal(unsigned long long value) const {
struct conv {
int val;
std::string rep;
};
static const conv table[] = {
{ 1000, "M" },
{ 900, "CM" },
{ 500, "D" },
{ 400, "CD" },
{ 100, "C" },
{ 90, "XC" },
{ 50, "L" },
{ 40, "XL" },
{ 10, "X" },
{ 9, "IX" },
{ 5, "V" },
{ 4, "IV" },
{ 1, "I" }
};
std::string roman;
for (auto const &c : table) {
while (value >= c.val) {
roman += c.rep;
value -= c.val;
}
}
return roman;
}
template
Iter fmt(T t, Iter i) const {
Solution
Using Iterator
Rather than building and passing a string around.
Why not use the output iterator directly?
You build a string letter by letter in the order you would output it, then you move that back to the calling function, then you copy the string to a stream (via an iterator). You can bypass the move and the copy by just writing the letters to the iterator.
Output Iterator not passed through:
OR not making it a template class.
Member Types
Your base type defines some member types.
This is exactly like your class. Usually I would expect you to pull these types from the parent rather than defining them independently ( unless there is a good reason).
So I would have expected:
Return value
Should this not be:
It still seems to work (even when the stream is std::stringstream), but I am a tiny bit surprised. You are supposed to return the value of the iterator after putting stuff on the stream (not the original value). It seems to work anyway because the iterator changes the state of the object that it is putting stuff into.
Rather than building and passing a string around.
std::string fmt_internal(unsigned long long value) const;Why not use the output iterator directly?
You build a string letter by letter in the order you would output it, then you move that back to the calling function, then you copy the string to a stream (via an iterator). You can bypass the move and the copy by just writing the letters to the iterator.
Output Iterator not passed through:
template >
class roman : public std::num_put {
// Expecting => ^^^^^^ OR not making it a template class.
class roman : public std::num_put {Member Types
Your base type defines some member types.
char_type CharT
iter_type OutputItThis is exactly like your class. Usually I would expect you to pull these types from the parent rather than defining them independently ( unless there is a good reason).
So I would have expected:
typedef std::num_put parent;
typedef parent::char_type char_type;
typedef parent::iter_type iter_type;Return value
std::copy(s.begin(), s.end(), i);
return i;Should this not be:
i = std::copy(s.begin(), s.end(), i);
return i;It still seems to work (even when the stream is std::stringstream), but I am a tiny bit surprised. You are supposed to return the value of the iterator after putting stuff on the stream (not the original value). It seems to work anyway because the iterator changes the state of the object that it is putting stuff into.
Code Snippets
std::string fmt_internal(unsigned long long value) const;template <class charT, class OutputIterator = std::ostreambuf_iterator<charT> >
class roman : public std::num_put < char > {
// Expecting => ^^^^^^ <CharT, OutputIterator>class roman : public std::num_put<char> {char_type CharT
iter_type OutputIttypedef std::num_put<CharT, OutputIterator> parent;
typedef parent::char_type char_type;
typedef parent::iter_type iter_type;Context
StackExchange Code Review Q#134923, answer score: 4
Revisions (0)
No revisions yet.