Function that adds commas between groups of 3 digits in a string
up vote
0
down vote
favorite
My program contains some very large integers and uses the GMP library. The output contains integers (probably small enough to fit into int
). I would like to make the output easy to read by adding commas between every three digits with any left overs on the left side.
Examples:
1234 => 1,234
12345 => 12,345
123456 => 123,456
123 => 123
The input of the function would be a string
representing an integer, because the value of the mpz_class
would be converted to a string. I know there's been a lot of discussion of adding commas to int
but not string
. The mpz_class
can't be converted to an int
because it may be too big.
Here is what I have and it isn't pretty.
using namespace std;
/*add commas between groups of 3 digits with remainder on left side*/
string addCommas(string in)
{
const int length = in.length();
if(length < 4)
{
return in;
}
int inserted = 0;
int i = length % 3;
if(i == 0)
{
i = 3;
}
for(; i < length + inserted; i = i + 4)
{
in.insert(i, ",");
inserted++;
}
return in;
}
Given its use in the program, it is guaranteed the input will not be a negative number. Though I still wonder if it's good to include a check? The program focuses on quick results. How can this code be more readable?
c++ formatting integer
add a comment |
up vote
0
down vote
favorite
My program contains some very large integers and uses the GMP library. The output contains integers (probably small enough to fit into int
). I would like to make the output easy to read by adding commas between every three digits with any left overs on the left side.
Examples:
1234 => 1,234
12345 => 12,345
123456 => 123,456
123 => 123
The input of the function would be a string
representing an integer, because the value of the mpz_class
would be converted to a string. I know there's been a lot of discussion of adding commas to int
but not string
. The mpz_class
can't be converted to an int
because it may be too big.
Here is what I have and it isn't pretty.
using namespace std;
/*add commas between groups of 3 digits with remainder on left side*/
string addCommas(string in)
{
const int length = in.length();
if(length < 4)
{
return in;
}
int inserted = 0;
int i = length % 3;
if(i == 0)
{
i = 3;
}
for(; i < length + inserted; i = i + 4)
{
in.insert(i, ",");
inserted++;
}
return in;
}
Given its use in the program, it is guaranteed the input will not be a negative number. Though I still wonder if it's good to include a check? The program focuses on quick results. How can this code be more readable?
c++ formatting integer
There's no definition for yourstring
type. Is it very similar tostd::string
(if so, why not usestd::string
?)
– Toby Speight
yesterday
@TobySpeight forgot to include theusing namespace std;
– northerner
yesterday
As a side note, I suspect commas are only inserted in numbers of five or more digits.
– bipll
yesterday
The streams already do this automatically. The "C" local has no commas. But most locals have an explicit point where commas go in a number. Thus by setting the correct local in your application you don't need to do anything else.
– Martin York
yesterday
add a comment |
up vote
0
down vote
favorite
up vote
0
down vote
favorite
My program contains some very large integers and uses the GMP library. The output contains integers (probably small enough to fit into int
). I would like to make the output easy to read by adding commas between every three digits with any left overs on the left side.
Examples:
1234 => 1,234
12345 => 12,345
123456 => 123,456
123 => 123
The input of the function would be a string
representing an integer, because the value of the mpz_class
would be converted to a string. I know there's been a lot of discussion of adding commas to int
but not string
. The mpz_class
can't be converted to an int
because it may be too big.
Here is what I have and it isn't pretty.
using namespace std;
/*add commas between groups of 3 digits with remainder on left side*/
string addCommas(string in)
{
const int length = in.length();
if(length < 4)
{
return in;
}
int inserted = 0;
int i = length % 3;
if(i == 0)
{
i = 3;
}
for(; i < length + inserted; i = i + 4)
{
in.insert(i, ",");
inserted++;
}
return in;
}
Given its use in the program, it is guaranteed the input will not be a negative number. Though I still wonder if it's good to include a check? The program focuses on quick results. How can this code be more readable?
c++ formatting integer
My program contains some very large integers and uses the GMP library. The output contains integers (probably small enough to fit into int
). I would like to make the output easy to read by adding commas between every three digits with any left overs on the left side.
Examples:
1234 => 1,234
12345 => 12,345
123456 => 123,456
123 => 123
The input of the function would be a string
representing an integer, because the value of the mpz_class
would be converted to a string. I know there's been a lot of discussion of adding commas to int
but not string
. The mpz_class
can't be converted to an int
because it may be too big.
Here is what I have and it isn't pretty.
using namespace std;
/*add commas between groups of 3 digits with remainder on left side*/
string addCommas(string in)
{
const int length = in.length();
if(length < 4)
{
return in;
}
int inserted = 0;
int i = length % 3;
if(i == 0)
{
i = 3;
}
for(; i < length + inserted; i = i + 4)
{
in.insert(i, ",");
inserted++;
}
return in;
}
Given its use in the program, it is guaranteed the input will not be a negative number. Though I still wonder if it's good to include a check? The program focuses on quick results. How can this code be more readable?
c++ formatting integer
c++ formatting integer
edited yesterday
200_success
127k15149412
127k15149412
asked yesterday
northerner
17815
17815
There's no definition for yourstring
type. Is it very similar tostd::string
(if so, why not usestd::string
?)
– Toby Speight
yesterday
@TobySpeight forgot to include theusing namespace std;
– northerner
yesterday
As a side note, I suspect commas are only inserted in numbers of five or more digits.
– bipll
yesterday
The streams already do this automatically. The "C" local has no commas. But most locals have an explicit point where commas go in a number. Thus by setting the correct local in your application you don't need to do anything else.
– Martin York
yesterday
add a comment |
There's no definition for yourstring
type. Is it very similar tostd::string
(if so, why not usestd::string
?)
– Toby Speight
yesterday
@TobySpeight forgot to include theusing namespace std;
– northerner
yesterday
As a side note, I suspect commas are only inserted in numbers of five or more digits.
– bipll
yesterday
The streams already do this automatically. The "C" local has no commas. But most locals have an explicit point where commas go in a number. Thus by setting the correct local in your application you don't need to do anything else.
– Martin York
yesterday
There's no definition for your
string
type. Is it very similar to std::string
(if so, why not use std::string
?)– Toby Speight
yesterday
There's no definition for your
string
type. Is it very similar to std::string
(if so, why not use std::string
?)– Toby Speight
yesterday
@TobySpeight forgot to include the
using namespace std;
– northerner
yesterday
@TobySpeight forgot to include the
using namespace std;
– northerner
yesterday
As a side note, I suspect commas are only inserted in numbers of five or more digits.
– bipll
yesterday
As a side note, I suspect commas are only inserted in numbers of five or more digits.
– bipll
yesterday
The streams already do this automatically. The "C" local has no commas. But most locals have an explicit point where commas go in a number. Thus by setting the correct local in your application you don't need to do anything else.
– Martin York
yesterday
The streams already do this automatically. The "C" local has no commas. But most locals have an explicit point where commas go in a number. Thus by setting the correct local in your application you don't need to do anything else.
– Martin York
yesterday
add a comment |
2 Answers
2
active
oldest
votes
up vote
2
down vote
Avoid using namespace std
The standard namespace isn't designed to be imported wholesale, and name collisions could later appear that silently change the meaning of your program (such as when you add an extra include, or move to a newer C++ standard).
std::string::insert
moves content multiple times
We're not very efficient, because the later characters will be shuffled as many times as there are preceding commas inserted. Worse, we don't even reserve()
enough space to add the commas, so there could (theoretically at least) be multiple memory allocations.
Don't reinvent the wheel
It would be simpler and more flexible to use std::numpunct
to perform the formatting.
add a comment |
up vote
0
down vote
You should not do this manually.
The local do this for you correctly for the local you are currently in (with the correct separation character for the locale).
The problem is that by default your code runs in the "C" locale. This uses no seporators. But you can make your code pick up the local used by the current machine.
#include <iostream>
#include <locale>
#include <string>
int main()
{
std::cout.imbue(std::locale(""));
std::cout << 123456789 << "n";
}
The output of this is:
123,456,789
In Germany it would look like:
123 456.789 // apparently correct separation for DE
Alternatively if you want your own specific separation you can set up the locale object on the stream to do the work specific to your application.
#include <iostream>
#include <locale>
#include <string>
template<typename CharT>
struct Sep : public std::numpunct<CharT>
{
virtual std::string do_grouping() const {return "03";}
virtual CharT do_thousands_sep() const {return ':';}
};
int main()
{
std::cout.imbue(std::locale(std::cout.getloc(), new Sep <char>()));
std::cout << 123456789 << "n"; // this prints 123:456:789
}
This outputs:
123:456:789
It would appearimbue(std::locale(""))
does not work on my machine. I copy and pasted your example and it had printed123456789
. I also copied the example here and still have no commas. I assume it wasn't working with the string returned from thempz_class
but it seems like something more fundamental is wrong.
– northerner
20 hours ago
Are there any special options needed for compiling with this?
– northerner
20 hours ago
@northerner: This means your machine has no current local (or your machines locale is "C"). You should probably look at the machine configuration and make sure it is correctly configured. But You can force a locale by setting the environment variable LC_ALL. Example:> LC_ALL=en_US ./myProgram
– Martin York
12 hours ago
There is nothing special you need to do. The locale code will ask the OS what the current locale is. So if the OS does not know then you will not get any change. But usually most machines are configured with a current locale (or at least a location and default language setting).
– Martin York
12 hours ago
add a comment |
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
2
down vote
Avoid using namespace std
The standard namespace isn't designed to be imported wholesale, and name collisions could later appear that silently change the meaning of your program (such as when you add an extra include, or move to a newer C++ standard).
std::string::insert
moves content multiple times
We're not very efficient, because the later characters will be shuffled as many times as there are preceding commas inserted. Worse, we don't even reserve()
enough space to add the commas, so there could (theoretically at least) be multiple memory allocations.
Don't reinvent the wheel
It would be simpler and more flexible to use std::numpunct
to perform the formatting.
add a comment |
up vote
2
down vote
Avoid using namespace std
The standard namespace isn't designed to be imported wholesale, and name collisions could later appear that silently change the meaning of your program (such as when you add an extra include, or move to a newer C++ standard).
std::string::insert
moves content multiple times
We're not very efficient, because the later characters will be shuffled as many times as there are preceding commas inserted. Worse, we don't even reserve()
enough space to add the commas, so there could (theoretically at least) be multiple memory allocations.
Don't reinvent the wheel
It would be simpler and more flexible to use std::numpunct
to perform the formatting.
add a comment |
up vote
2
down vote
up vote
2
down vote
Avoid using namespace std
The standard namespace isn't designed to be imported wholesale, and name collisions could later appear that silently change the meaning of your program (such as when you add an extra include, or move to a newer C++ standard).
std::string::insert
moves content multiple times
We're not very efficient, because the later characters will be shuffled as many times as there are preceding commas inserted. Worse, we don't even reserve()
enough space to add the commas, so there could (theoretically at least) be multiple memory allocations.
Don't reinvent the wheel
It would be simpler and more flexible to use std::numpunct
to perform the formatting.
Avoid using namespace std
The standard namespace isn't designed to be imported wholesale, and name collisions could later appear that silently change the meaning of your program (such as when you add an extra include, or move to a newer C++ standard).
std::string::insert
moves content multiple times
We're not very efficient, because the later characters will be shuffled as many times as there are preceding commas inserted. Worse, we don't even reserve()
enough space to add the commas, so there could (theoretically at least) be multiple memory allocations.
Don't reinvent the wheel
It would be simpler and more flexible to use std::numpunct
to perform the formatting.
answered yesterday
Toby Speight
23.2k538110
23.2k538110
add a comment |
add a comment |
up vote
0
down vote
You should not do this manually.
The local do this for you correctly for the local you are currently in (with the correct separation character for the locale).
The problem is that by default your code runs in the "C" locale. This uses no seporators. But you can make your code pick up the local used by the current machine.
#include <iostream>
#include <locale>
#include <string>
int main()
{
std::cout.imbue(std::locale(""));
std::cout << 123456789 << "n";
}
The output of this is:
123,456,789
In Germany it would look like:
123 456.789 // apparently correct separation for DE
Alternatively if you want your own specific separation you can set up the locale object on the stream to do the work specific to your application.
#include <iostream>
#include <locale>
#include <string>
template<typename CharT>
struct Sep : public std::numpunct<CharT>
{
virtual std::string do_grouping() const {return "03";}
virtual CharT do_thousands_sep() const {return ':';}
};
int main()
{
std::cout.imbue(std::locale(std::cout.getloc(), new Sep <char>()));
std::cout << 123456789 << "n"; // this prints 123:456:789
}
This outputs:
123:456:789
It would appearimbue(std::locale(""))
does not work on my machine. I copy and pasted your example and it had printed123456789
. I also copied the example here and still have no commas. I assume it wasn't working with the string returned from thempz_class
but it seems like something more fundamental is wrong.
– northerner
20 hours ago
Are there any special options needed for compiling with this?
– northerner
20 hours ago
@northerner: This means your machine has no current local (or your machines locale is "C"). You should probably look at the machine configuration and make sure it is correctly configured. But You can force a locale by setting the environment variable LC_ALL. Example:> LC_ALL=en_US ./myProgram
– Martin York
12 hours ago
There is nothing special you need to do. The locale code will ask the OS what the current locale is. So if the OS does not know then you will not get any change. But usually most machines are configured with a current locale (or at least a location and default language setting).
– Martin York
12 hours ago
add a comment |
up vote
0
down vote
You should not do this manually.
The local do this for you correctly for the local you are currently in (with the correct separation character for the locale).
The problem is that by default your code runs in the "C" locale. This uses no seporators. But you can make your code pick up the local used by the current machine.
#include <iostream>
#include <locale>
#include <string>
int main()
{
std::cout.imbue(std::locale(""));
std::cout << 123456789 << "n";
}
The output of this is:
123,456,789
In Germany it would look like:
123 456.789 // apparently correct separation for DE
Alternatively if you want your own specific separation you can set up the locale object on the stream to do the work specific to your application.
#include <iostream>
#include <locale>
#include <string>
template<typename CharT>
struct Sep : public std::numpunct<CharT>
{
virtual std::string do_grouping() const {return "03";}
virtual CharT do_thousands_sep() const {return ':';}
};
int main()
{
std::cout.imbue(std::locale(std::cout.getloc(), new Sep <char>()));
std::cout << 123456789 << "n"; // this prints 123:456:789
}
This outputs:
123:456:789
It would appearimbue(std::locale(""))
does not work on my machine. I copy and pasted your example and it had printed123456789
. I also copied the example here and still have no commas. I assume it wasn't working with the string returned from thempz_class
but it seems like something more fundamental is wrong.
– northerner
20 hours ago
Are there any special options needed for compiling with this?
– northerner
20 hours ago
@northerner: This means your machine has no current local (or your machines locale is "C"). You should probably look at the machine configuration and make sure it is correctly configured. But You can force a locale by setting the environment variable LC_ALL. Example:> LC_ALL=en_US ./myProgram
– Martin York
12 hours ago
There is nothing special you need to do. The locale code will ask the OS what the current locale is. So if the OS does not know then you will not get any change. But usually most machines are configured with a current locale (or at least a location and default language setting).
– Martin York
12 hours ago
add a comment |
up vote
0
down vote
up vote
0
down vote
You should not do this manually.
The local do this for you correctly for the local you are currently in (with the correct separation character for the locale).
The problem is that by default your code runs in the "C" locale. This uses no seporators. But you can make your code pick up the local used by the current machine.
#include <iostream>
#include <locale>
#include <string>
int main()
{
std::cout.imbue(std::locale(""));
std::cout << 123456789 << "n";
}
The output of this is:
123,456,789
In Germany it would look like:
123 456.789 // apparently correct separation for DE
Alternatively if you want your own specific separation you can set up the locale object on the stream to do the work specific to your application.
#include <iostream>
#include <locale>
#include <string>
template<typename CharT>
struct Sep : public std::numpunct<CharT>
{
virtual std::string do_grouping() const {return "03";}
virtual CharT do_thousands_sep() const {return ':';}
};
int main()
{
std::cout.imbue(std::locale(std::cout.getloc(), new Sep <char>()));
std::cout << 123456789 << "n"; // this prints 123:456:789
}
This outputs:
123:456:789
You should not do this manually.
The local do this for you correctly for the local you are currently in (with the correct separation character for the locale).
The problem is that by default your code runs in the "C" locale. This uses no seporators. But you can make your code pick up the local used by the current machine.
#include <iostream>
#include <locale>
#include <string>
int main()
{
std::cout.imbue(std::locale(""));
std::cout << 123456789 << "n";
}
The output of this is:
123,456,789
In Germany it would look like:
123 456.789 // apparently correct separation for DE
Alternatively if you want your own specific separation you can set up the locale object on the stream to do the work specific to your application.
#include <iostream>
#include <locale>
#include <string>
template<typename CharT>
struct Sep : public std::numpunct<CharT>
{
virtual std::string do_grouping() const {return "03";}
virtual CharT do_thousands_sep() const {return ':';}
};
int main()
{
std::cout.imbue(std::locale(std::cout.getloc(), new Sep <char>()));
std::cout << 123456789 << "n"; // this prints 123:456:789
}
This outputs:
123:456:789
answered yesterday
Martin York
72.2k482256
72.2k482256
It would appearimbue(std::locale(""))
does not work on my machine. I copy and pasted your example and it had printed123456789
. I also copied the example here and still have no commas. I assume it wasn't working with the string returned from thempz_class
but it seems like something more fundamental is wrong.
– northerner
20 hours ago
Are there any special options needed for compiling with this?
– northerner
20 hours ago
@northerner: This means your machine has no current local (or your machines locale is "C"). You should probably look at the machine configuration and make sure it is correctly configured. But You can force a locale by setting the environment variable LC_ALL. Example:> LC_ALL=en_US ./myProgram
– Martin York
12 hours ago
There is nothing special you need to do. The locale code will ask the OS what the current locale is. So if the OS does not know then you will not get any change. But usually most machines are configured with a current locale (or at least a location and default language setting).
– Martin York
12 hours ago
add a comment |
It would appearimbue(std::locale(""))
does not work on my machine. I copy and pasted your example and it had printed123456789
. I also copied the example here and still have no commas. I assume it wasn't working with the string returned from thempz_class
but it seems like something more fundamental is wrong.
– northerner
20 hours ago
Are there any special options needed for compiling with this?
– northerner
20 hours ago
@northerner: This means your machine has no current local (or your machines locale is "C"). You should probably look at the machine configuration and make sure it is correctly configured. But You can force a locale by setting the environment variable LC_ALL. Example:> LC_ALL=en_US ./myProgram
– Martin York
12 hours ago
There is nothing special you need to do. The locale code will ask the OS what the current locale is. So if the OS does not know then you will not get any change. But usually most machines are configured with a current locale (or at least a location and default language setting).
– Martin York
12 hours ago
It would appear
imbue(std::locale(""))
does not work on my machine. I copy and pasted your example and it had printed 123456789
. I also copied the example here and still have no commas. I assume it wasn't working with the string returned from the mpz_class
but it seems like something more fundamental is wrong.– northerner
20 hours ago
It would appear
imbue(std::locale(""))
does not work on my machine. I copy and pasted your example and it had printed 123456789
. I also copied the example here and still have no commas. I assume it wasn't working with the string returned from the mpz_class
but it seems like something more fundamental is wrong.– northerner
20 hours ago
Are there any special options needed for compiling with this?
– northerner
20 hours ago
Are there any special options needed for compiling with this?
– northerner
20 hours ago
@northerner: This means your machine has no current local (or your machines locale is "C"). You should probably look at the machine configuration and make sure it is correctly configured. But You can force a locale by setting the environment variable LC_ALL. Example:
> LC_ALL=en_US ./myProgram
– Martin York
12 hours ago
@northerner: This means your machine has no current local (or your machines locale is "C"). You should probably look at the machine configuration and make sure it is correctly configured. But You can force a locale by setting the environment variable LC_ALL. Example:
> LC_ALL=en_US ./myProgram
– Martin York
12 hours ago
There is nothing special you need to do. The locale code will ask the OS what the current locale is. So if the OS does not know then you will not get any change. But usually most machines are configured with a current locale (or at least a location and default language setting).
– Martin York
12 hours ago
There is nothing special you need to do. The locale code will ask the OS what the current locale is. So if the OS does not know then you will not get any change. But usually most machines are configured with a current locale (or at least a location and default language setting).
– Martin York
12 hours ago
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209063%2ffunction-that-adds-commas-between-groups-of-3-digits-in-a-string%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
There's no definition for your
string
type. Is it very similar tostd::string
(if so, why not usestd::string
?)– Toby Speight
yesterday
@TobySpeight forgot to include the
using namespace std;
– northerner
yesterday
As a side note, I suspect commas are only inserted in numbers of five or more digits.
– bipll
yesterday
The streams already do this automatically. The "C" local has no commas. But most locals have an explicit point where commas go in a number. Thus by setting the correct local in your application you don't need to do anything else.
– Martin York
yesterday