packing an integer?

sorry for another question about this, but this one is different.

i wrote a script earlier to read and write 4 byte integers to files, now i'm working on one to read and write 2 byte intgers. can anyone tell me what i did wrong with this script?

Code:

---

void bin_write_2bytes(ofstream *file, unsigned int value)
{
    unsigned char byteA = (value/253) + 1;
    unsigned char byteB = (value%253) + 1;

    *file << byteA;
    *file << byteB;
}

unsigned int bin_read_2bytes(ifstream *file)
{
    unsigned char byteA = file->get()*253;
    unsigned char byteB = file->get();

    unsigned int value = byteA + byteB;
    return (value);
}

---

i know the script is way off because i tried saving 61921 and it read back 223. thank you for your time.

Code:

---

unsigned char byteA = file->get()*253;

---

First, why 253? Second, if an unsigned char may (usually) have values from 0-255, what values do you think bytaA will end up holding?

Let's see ...

First, the write function is wrong. Or let's rather say, unsafe. You're using formatted text output, when you should use unformatted output. Use put on the stream.

Second, your radix is wrong. An 8-bit unsigned byte can contain the numbers from 0 to 255, so your calculation value must be 256, not 253. Also, the +1 is wrong.

Third, while it works, division, modulo and addition are not the typical way of splitting integers. This is done using bit masking and shifting, like this:

Code:

---

unsigned char byteA = static_cast<unsigned char>((value >> 8) & 0xFF);
unsigned char byteB = static_cast<unsigned char>(value & 0xFF);

---

Code:

---

value = (static_cast<unsigned int>(byteA) << 8) | byteB;

---

Fourth, it's probably a better idea to pass the streams by reference instead of pointer.

Fifth, I just noticed, you are overflowing byteA on reading by multiplying the value first and then trying to assign it to the byte. But the multiplied value is almost guaranteed (almost because of your incorrect radix) to overflow byteA, causing it to "wrap around" and start counting from zero again. I'm pretty sure this is the main error in your code, as far as the observed effect goes.

ok. thank you both for all your help. thats what i get for trying to take a script from an interpreted language and change it to c++ without thinking. i thought something had to be wrong with byteA but, i figured since it worked in the other language it would work in c++.

anyways I wanted to do this without bit shifting because isn't bit shifting a lot slower than simple multiplacation and devision? the 4 byte script i wrote uses bit shifting because i couldn't think of another way to do it.

finally, why is it a better idea to pass the stream than a pointer?

anyways, here is the code as of now. if you see an error please let me know. it appears to be working correctly.

Code:

---

void bin_write_2bytes(ofstream *file, unsigned int value)
{
    unsigned char byteA = (value/256);
    unsigned char byteB = (value%256);

    *file << byteA;
    *file << byteB;
}

unsigned int bin_read_2bytes(ifstream *file)
{
    unsigned int byteA = file->get()*256;
    unsigned char byteB = file->get();

    unsigned int value = byteA + byteB;
    return (value);
}

---

Bit shifting is faaaaar faster than division. It's a good deal faster than multiplication. And it was even more so in old times.

BTW, is this just for learning or do you actually need it? Because if you need it, well, there's a write() and a read() method of the streams that can do this stuff somewhat better than any hand-written stuff.

no, i actually need it. but i'm learning from it also.

i tried using bit shifting and it actaully turned out easier than i thought. now i'm interested in read() and write(). i guess i'll have a look at those. thank you for your help.