Simplify this ?

[Flzw]

Alright, here is a simple function I coded, won't be hard understanding what
it does, and YES, I know, you will probably tell me it's awful code, I would
like to know how to write it better, maybe using stringstream ? here it is :

std::string IntToDiskSpace( int size)
{
if (size < 1024)
return IntToStr( size ) + " Bytes";
else if (size < 1048576)
return IntToStr( size / 1024) + "KB";
else if (size < 1073741824)
{
char* temp = new char[64];
sprintf( temp, "%2f", size / 1048576);
string result = temp;
result += " MB";
delete temp;
return result;
}
else
{
char* temp = new char[64];
sprintf( temp, "%2f", size / 1073741824);
string result = temp;
result += " GB";
delete temp;
return result;
}
}

[Mike Wahler]

"Flzw" <fl****@wanadoo.fr> wrote in message
news:41**********************@news.wanadoo.fr...

Alright, here is a simple function I coded, won't be hard understanding what it does,
No, we cannot know what it does, since it calls a function
whose definition you don't show.
and YES, I know, you will probably tell me it's awful code,
Well, it's incomplete code. It won't compile. If you want
folks to assess it, it's much better to post something compilable
if at all possible.
I would
like to know how to write it better, maybe using stringstream ? here it is

:

I suggest you tell us, in English, what you want it to do.
And yes, I'd get rid of the e.g. 'sprintf()'s as there
are safer, more robust ways to generate strings.

I'd also dump the unnecessary dynamic allocations.

-Mike

[Flzw]

Allright, as Mike pointed out, there was not the code for another function I
made but that was irrelevant in the questions I had but yes I agree that was
not necessarily obvious, so let's reduce the code to this :

std::string IntToDiskSpace( int size)
{
if (size < 1073741824)
{
char* temp = new char[64];
sprintf( temp, "%2f", size / 1048576);
string result = temp;
result += " MB";
delete temp;
return result;
}
else
{
char* temp = new char[64];
sprintf( temp, "%2f", size / 1073741824);
string result = temp;
result += " GB";
delete temp;
return result;
}
}

I know I shouldn't use sprintf (thus the "unecessary dynamic allocations"
but I'm unsure on how to dit, I guess stringstream or stringbuf could help
there but these are quite obscure for me...

[Victor Bazarov]

"Flzw" <fl****@wanadoo.fr> wrote...

Alright, here is a simple function I coded, won't be hard understanding
what
it does, and YES, I know, you will probably tell me it's awful code,
It's not awful. It's just full of assumptions which are not necessarily
correct, and some undefined behaviour. Just a little bit of undefined
behaviour, not a whole lot. Well, actually, more than a little bit.
Yeah, it's awful.
I would
like to know how to write it better, maybe using stringstream ? here it is
:

std::string IntToDiskSpace( int size)
{
if (size < 1024)
return IntToStr( size ) + " Bytes";
else if (size < 1048576)
return IntToStr( size / 1024) + "KB";
else if (size < 1073741824)
1073741824 is a literal that doesn't necessarily fit into an int. You might
want to consider using 'L' suffix.
{
char* temp = new char[64];
You really don't need to dynamically allocate 'temp'. Just declare the
array as automatic:

char temp[64];
sprintf( temp, "%2f", size / 1048576);
You probably wanted to print decimal points, the format you need is '%.2f'.
Second, the 'size / 1048576' has the type 'int', so if you have %f for
the format, the 'sprintf' expects a double value. So, you _need_ to supply
a double value there, otherwise its behaviour is undefined:

sprintf(temp, "%.2f", size / 1048576.);
string result = temp;
result += " MB";
delete temp;
Another bit of undefined behaviour. If you allocate using 'new[]', you
MUST free it using 'delete[]':

delete[] temp;

Of course, if you follow my advice to use a simple automatic array, you
don't need to delete at all.
return result;
}
else
{
Same notes in this block as above.
char* temp = new char[64];
sprintf( temp, "%2f", size / 1073741824);
string result = temp;
result += " GB";
delete temp;
return result;
}
}

A simplified version might begin like this

std::ostringstream os;
if (size < 1024)
return (os << size << " Bytes").str();
size /= 1024;
if (size < 1024)
return (os << size << " KB").str();
...

Victor

[Alf P. Steinbach]

* Flzw:

Alright, here is a simple function I coded, won't be hard understanding what
it does, and YES, I know, you will probably tell me it's awful code, I would
like to know how to write it better, maybe using stringstream ? here it is :
First thing is to make it _correct_ as per current standards.

1024 bytes is not 1 KB. It's 1 KiB.

2^20 bytes is not 1 MB. It's 1 MiB.

std::string IntToDiskSpace( int size)
'int' is unlikely to be able to represent modern disk capacitites.

{
if (size < 1024)
return IntToStr( size ) + " Bytes";
else if (size < 1048576)
return IntToStr( size / 1024) + "KB";
else if (size < 1073741824)
Instead of decimal numbers, use values derived from expressions
such as

size_t const K = (1 << 10); // 1024
size_t const M = K*K;
size_t const G = K*M;
{
char* temp = new char[64];
sprintf( temp, "%2f", size / 1048576);

Oops.

Check out boost::lexical_cast. See <url: http://www.boost.org>.

When you get things _working_ and _correct_ you can start to
worry about optimization -- until then forget optimization!

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[Victor Bazarov]

"Flzw" <fl****@wanadoo.fr> wrote...

Allright, as Mike pointed out, there was not the code for another function
I made but that was irrelevant in the questions I had but yes I agree that
was not necessarily obvious, so let's reduce the code to this :

std::string IntToDiskSpace( int size)
{
if (size < 1073741824)
Another advice (in addition to my post): typos are programmer's worst
enemies.
It's quite possible to make a mistake in such a constant even if you copy
and
paste it from a calculator or something. I recommend using 1024*1024*1024
instead. The compiler will calculate it for you. Besides, it's readable a
bit
better than 1034234582 (oops.. I guess I mistyped)
[...]

[Victor Bazarov]

"Alf P. Steinbach" <al***@start.no> wrote...

* Flzw:

Alright, here is a simple function I coded, won't be hard understanding
what
it does, and YES, I know, you will probably tell me it's awful code, I
would
like to know how to write it better, maybe using stringstream ? here it
is :
First thing is to make it _correct_ as per current standards.

1024 bytes is not 1 KB. It's 1 KiB.

2^20 bytes is not 1 MB. It's 1 MiB.

And which OS do you know that reports file sizes in "KiB" or "MiB"?
Besides, isn't "MiB" a trademark or something for "Men In Black, The"?

std::string IntToDiskSpace( int size)

'int' is unlikely to be able to represent modern disk capacitites.

{
if (size < 1024)
return IntToStr( size ) + " Bytes";
else if (size < 1048576)
return IntToStr( size / 1024) + "KB";
else if (size < 1073741824)

Instead of decimal numbers, use values derived from expressions
such as

size_t const K = (1 << 10); // 1024
size_t const M = K*K;
size_t const G = K*M;

{
char* temp = new char[64];
sprintf( temp, "%2f", size / 1048576);

Oops.

Check out boost::lexical_cast. See <url: http://www.boost.org>.

Why? Couldn't _standard_ means be used?
When you get things _working_ and _correct_ you can start to
worry about optimization -- until then forget optimization!

He didn't ask for optimization. He asked to simplify. That's mostly
related to _readability_ and _maintainability_ and not optimization.

V

[Alf P. Steinbach]

* Victor Bazarov:

"Alf P. Steinbach" <al***@start.no> wrote...

* Flzw:

Alright, here is a simple function I coded, won't be hard understanding
what
it does, and YES, I know, you will probably tell me it's awful code, I
would
like to know how to write it better, maybe using stringstream ? here it
is :
First thing is to make it _correct_ as per current standards.

1024 bytes is not 1 KB. It's 1 KiB.

2^20 bytes is not 1 MB. It's 1 MiB.

And which OS do you know that reports file sizes in "KiB" or "MiB"?

Some Linux utilities and desktops do, not that I know them intimately
or can provide references out of hand.

The situation for Windows is, however, as always: everything's
non-standard, including the terminology, units, etc.

For simple information about the international standard, see
<url: http://physics.nist.gov/cuu/Units/binary.html>.

Besides, isn't "MiB" a trademark or something for "Men In Black, The"?
He he he... :-)

You got me laughing.

Somebody should notify Hollywood -- great lawsuit!

Check out boost::lexical_cast. See <url: http://www.boost.org>.

Why? Couldn't _standard_ means be used?

boost::lexical_cast is a simple wrapper around the standard means,
namely a std::stringstream-based conversion -- check it out... ;-)

When you get things _working_ and _correct_ you can start to
worry about optimization -- until then forget optimization!

He didn't ask for optimization. He asked to simplify. That's mostly
related to _readability_ and _maintainability_

Yes. We agree! :-)
and not optimization.

Nope. All the code's ugliness stems from misguided attempts at
optimization, e.g. using sprintf instead of iostreams. Where the
possible benefit is more than cancelled by the overhead of 'new'.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[Flzw]

It's not awful. It's just full of assumptions which are not necessarily
correct, and some undefined behaviour. Just a little bit of undefined
behaviour, not a whole lot. Well, actually, more than a little bit.
Yeah, it's awful.

Thought so ;)

1073741824 is a literal that doesn't necessarily fit into an int. You
might
want to consider using 'L' suffix.
Well, it's only formy own use on a personal project compiled with a compiler
with 32bits ints, so it does fit in, I did this on purpose, but I changed
anyway, dividing by 1024 each time will probably be faster.
You really don't need to dynamically allocate 'temp'. Just declare the
array as automatic:

char temp[64];

good point, like the 2 others, we'll blame the delete[] stuff on fatigue

A simplified version might begin like this

std::ostringstream os;
if (size < 1024)
return (os << size << " Bytes").str();
size /= 1024;
if (size < 1024)
return (os << size << " KB").str();
...

Allrigth this is it, I tried something like this :
string IntToDiskSpace( int size)
{
std::ostringstream os;
double fsize = size;

if (fsize < 1024.0)
return (os << size << " Bytes").str();
fsize /= 1024.0;
if (fsize < 1024.0)
return (os << size / 1024 << " KB").str();
fsize /= 1024.0;
if (fsize < 1024.0)
return (os << ios::fixed() << setprecision(2) << fsize << " MB").str();
fsize /= 1024.0;
return (os << fixed << setprecision(2) << fsize << " GB").str();
}

There are probably mistakes in this too, but the compiler gives me one on
the first line, and I don't understand it :

error C2079: 'os' uses undefined class
'std::basic_ostringstream<_Elem,_Traits,_Alloc>'
with
[
_Elem=char,
_Traits=std::char_traits<char>,
_Alloc=std::allocator<char>
]

etc ...
I can't find why....

[Flzw]

error C2079: 'os' uses undefined class
'std::basic_ostringstream<_Elem,_Traits,_Alloc>'
with
[
_Elem=char,
_Traits=std::char_traits<char>,
_Alloc=std::allocator<char>
]

etc ...
I can't find why....

#include <sstream> right =)

[Karl Heinz Buchegger]

Flzw wrote:

There are probably mistakes in this too, but the compiler gives me one on
the first line, and I don't understand it :

error C2079: 'os' uses undefined class

Whenever you see 'uses undefined class' the compiler has
encountered a name which was not introduced to it earlier.

Basically there are 2 reasones for that.
You either
* have not included everything you should
or
* you made a simple typo.

Since the compiler complains about something not written
by yourself, it most likely is not a typo but a missing
include.

--
Karl Heinz Buchegger
kb******@gascad.at

[Howard]

>

2^20 bytes is not 1 MB. It's 1 MiB.

That's just plain weird! We're using "MiB" for "megabyte" now? Exactly
where did the "i" come from??? :-)

-Howard

[White Wolf]

Howard wrote:

2^20 bytes is not 1 MB. It's 1 MiB.

That's just plain weird! We're using "MiB" for "megabyte" now? Exactly
where did the "i" come from??? :-)

Man in Black. Yoy should go to movies more often. ;-) MB is 2^20bytes, it
is only the cheapo hard-disk manufacturers who are misusing it.

--
WW aka Attila
:::
Adding manpower to a late software project makes it later.

[Victor Bazarov]

Howard wrote:

2^20 bytes is not 1 MB. It's 1 MiB.

That's just plain weird! We're using "MiB" for "megabyte" now? Exactly
where did the "i" come from??? :-)

It's the 8th letter of the Latin alphabet, probably adopted by
Romans from the Greek "iota". So, a Mega-i-Byte is an iota bigger
than a MegaByte.

[Victor Bazarov]

Victor Bazarov wrote:

Howard wrote:

2^20 bytes is not 1 MB. It's 1 MiB.

That's just plain weird! We're using "MiB" for "megabyte" now?
Exactly where did the "i" come from??? :-)

It's the 8th letter of the Latin alphabet, probably adopted by
Romans from the Greek "iota". So, a Mega-i-Byte is an iota bigger
than a MegaByte.

Scratch that. Should read: a Mebibyte is an iota bigger than a Megabyte.

[Mike Wahler]

"Victor Bazarov" <v.********@comAcast.net> wrote in message
news:L-********************@comcast.com...

"Alf P. Steinbach" <al***@start.no> wrote...

1024 bytes is not 1 KB. It's 1 KiB.

2^20 bytes is not 1 MB. It's 1 MiB.

And which OS do you know that reports file sizes in "KiB" or "MiB"?
Besides, isn't "MiB" a trademark or something for "Men In Black, The"?

I suspect so (if I recall correctly, those guys were
in the business of controlling (alien) bugs.) :-)

-Mike

[Mike Wahler]

"Victor Bazarov" <v.********@comAcast.net> wrote in message
news:Ko*******************@newsread1.mlpsca01.us.t o.verio.net...

Victor Bazarov wrote:

Howard wrote:

2^20 bytes is not 1 MB. It's 1 MiB.

That's just plain weird! We're using "MiB" for "megabyte" now?
Exactly where did the "i" come from??? :-)

It's the 8th letter of the Latin alphabet, probably adopted by
Romans from the Greek "iota". So, a Mega-i-Byte is an iota bigger
than a MegaByte.

Scratch that. Should read: a Mebibyte is an iota bigger than a Megabyte.

Write a program to calculate and
output the value of an iota.

Don't use atoi().

:-)

-Mike

[Mike Wahler]

"Mike Wahler" <mk******@mkwahler.net> wrote in message
news:wb****************@newsread1.news.pas.earthli nk.net...

"Victor Bazarov" <v.********@comAcast.net> wrote in message

Scratch that. Should read: a Mebibyte is an iota bigger than a

Megabyte.
Write a program to calculate and
output the value of an iota.
Hmm, that probably works better if I'd
written "an iota".

Don't use atoi().

-Mike

[Julián Albo]

Howard wrote:

That's just plain weird! We're using "MiB" for "megabyte" now? Exactly
where did the "i" come from??? :-)

The mega prefix binary style is now called mebi, bi for binary.

See http://physics.nist.gov/cuu/Units/binary.html for example.

--
Salu2

[Old Wolf]

Victor Bazarov wrote:

std::ostringstream os;
if (size < 1024)
return (os << size << " Bytes").str();
size /= 1024;
if (size < 1024)
return (os << size << " KB").str();
...

That doesn't work. The operator<< functions are
in the basic_ostream class and return a value of
type basic_ostream<....> & , which has no member
function 'str'.

[Alan Krueger]

Victor Bazarov wrote:

"Flzw" <fl****@wanadoo.fr> wrote...

[...]

if (size < 1073741824)

Another advice (in addition to my post): typos are programmer's worst
enemies. It's quite possible to make a mistake in such a constant even
if you copy and paste it from a calculator or something. I recommend
using 1024*1024*1024 instead. The compiler will calculate it for you.
Besides, it's readable a bit better than 1034234582 (oops.. I guess
I mistyped)

Er, wouldn't it be better to use something more suitable to powers of
two, like hexadecimal?

1024 = 1024 = 0x400
1024*1024 = 1048576 = 0x100000
1024*1024*1024 = 1073741824 = 0x40000000

[Alan Krueger]

Julián Albo wrote:

The mega prefix binary style is now called mebi, bi for binary.

Personally, "mebibyte", "gibibyte", and "tebibyte" sound totally absurd.
I can't bring myself to say them.

[Victor Bazarov]

"Alan Krueger" <wg*******@sneakemail.com> wrote...

Victor Bazarov wrote:

"Flzw" <fl****@wanadoo.fr> wrote...

[...]

> if (size < 1073741824)

Another advice (in addition to my post): typos are programmer's worst
enemies. It's quite possible to make a mistake in such a constant even
if you copy and paste it from a calculator or something. I recommend
using 1024*1024*1024 instead. The compiler will calculate it for you.
Besides, it's readable a bit better than 1034234582 (oops.. I guess
I mistyped)

Er, wouldn't it be better to use something more suitable to powers of two,
like hexadecimal?

No.

[Alan Krueger]

Victor Bazarov wrote:

"Alan Krueger" <wg*******@sneakemail.com> wrote...

Er, wouldn't it be better to use something more suitable to powers of two,
like hexadecimal?

No.

Truly a well-considered, informative response.

To reuse an example, 0x40000000 is much easier to maintain than
1073541824. Without recalculating that number or looking it up, can you
spot the error I introduced typing in that number? If you can, you're
in a very small minority. By contrast, any change to the former would
be much more visible.

As an extreme example of this, I once saw a sequence of increasing
single-bit bitmasks encoded in decimal. Curiously, a typo had been
added by accident in the list, introducing odd behaviors elsewhere in
the code. Finding the typo was a pain in the ass, whereas encoding them
in hexadecimal would have made the error stand out and be immediately
detectable.

[Victor Bazarov]

"Alan Krueger" <wg*******@sneakemail.com> wrote...

Victor Bazarov wrote:

"Alan Krueger" <wg*******@sneakemail.com> wrote...

> Er, wouldn't it be better to use something more suitable to powers of
> two, like hexadecimal?
No.

Truly a well-considered, informative response.

To reuse an example, 0x40000000 is much easier to maintain than
1073541824.

Both 0x4000000000 and 1073541824 are worse than 1024*1024*1024. Period.
Without recalculating that number or looking it up, can you spot the
error I introduced typing in that number? If you can, you're in a very
small minority. By contrast, any change to the former would be much more
visible.
Nope. The number of zeros you have to type makes it just as bad.
[...]

[Old Wolf]

Victor Bazarov wrote:

"Alan Krueger" <wg*******@sneakemail.com> wrote...

Victor Bazarov wrote:

"Alan Krueger" <wg*******@sneakemail.com> wrote...
> Er, wouldn't it be better to use something more suitable to powers of > two, like hexadecimal?

No.

Truly a well-considered, informative response.

To reuse an example, 0x40000000 is much easier to maintain than
1073541824.

Both 0x4000000000 and 1073541824 are worse than 1024*1024*1024.

Period.

1024*1024*1024 is the worst of the three. On a 16-bit
system it will cause undefined behaviour (signed int
overflow).

[Old Wolf]

Victor Bazarov wrote:

"Alan Krueger" <wg*******@sneakemail.com> wrote...

Victor Bazarov wrote:

"Alan Krueger" <wg*******@sneakemail.com> wrote...
> Er, wouldn't it be better to use something more suitable to powers of > two, like hexadecimal?

No.

Truly a well-considered, informative response.

To reuse an example, 0x40000000 is much easier to maintain than
1073541824.

Both 0x4000000000 and 1073541824 are worse than 1024*1024*1024.

Period.

1024*1024*1024 is the worst of the three. On a 16-bit
system it will cause undefined behaviour (signed int
overflow).

[Matt Parkins]

"Victor Bazarov" <v.********@comAcast.net> wrote in message
news:wM********************@comcast.com...

"Alan Krueger" <wg*******@sneakemail.com> wrote...

Victor Bazarov wrote:

"Flzw" <fl****@wanadoo.fr> wrote...

[...]

> if (size < 1073741824)

Another advice (in addition to my post): typos are programmer's worst
enemies. It's quite possible to make a mistake in such a constant even
> if you copy and paste it from a calculator or something. I recommend
> using 1024*1024*1024 instead. The compiler will calculate it for you.
> Besides, it's readable a bit better than 1034234582 (oops.. I guess
> I mistyped)

Er, wouldn't it be better to use something more suitable to powers of
two, like hexadecimal?

No.

Alternatively you could use some little macros:

#define KILO(n) (n *1024)
#define MEGA(n) (n *(1024 *1024))
#define GIGA(n) (n *(1024 *1024 *1024))

if (size <GIGA(1))

or

if (size < MEGA(1024))

I'm just thinking readability really.

Matt

[Victor Bazarov]

"Matt Parkins" <no**********@idontthinksomyfriend.com> wrote...

"Victor Bazarov" <v.********@comAcast.net> wrote in message
news:wM********************@comcast.com...

"Alan Krueger" <wg*******@sneakemail.com> wrote...

Victor Bazarov wrote:
"Flzw" <fl****@wanadoo.fr> wrote...
[...]
> if (size < 1073741824)

Another advice (in addition to my post): typos are programmer's worst
enemies. It's quite possible to make a mistake in such a constant even
> if you copy and paste it from a calculator or something. I recommend
> using 1024*1024*1024 instead. The compiler will calculate it for you.
> Besides, it's readable a bit better than 1034234582 (oops.. I guess
> I mistyped)

Er, wouldn't it be better to use something more suitable to powers of
two, like hexadecimal?
No.

Alternatively you could use some little macros:

#define KILO(n) (n *1024)

#define KILO(n) ((n) *1024)
[...]

And we could actually try to set a trend if we call them

KIBI
MEBI
GIBI

:-)

V

[Matt Parkins]

>> #define KILO(n) (n *1024)

#define KILO(n) ((n) *1024)
Oh yeah, I forgot! :)
And we could actually try to set a trend if we call them

KIBI
MEBI
GIBI

:-)

What a great idea :@)