Sign in to follow this  

Code design in C++

This topic is 3725 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

First off, I have been programming in C++ for some time, but not really in the right way... so what I'm trying to do right now is to learn OO programing, as oposed to "C with classes". Basically I have a c_bitmap class that is a fancy array of the s_color struct. I can build it from another bitmap or from the ground up, and I can paint other bitmaps on top of it. But now I wanted to have a way to build my bitmap based on a filename. My initial idea was to have a overloaded constructor that simply take the filename and a bool by reference (bool& success), but I was wondering if that's a nice solution. Particulary, I was hoping to some day reuse this code in a PalmOS program, and I'm not shure if file reading works differently in that plataform. However, doesen't that breakes encapsulation? I mean, I keep thinking that my bitmap class shouldn't care about loading files, but if that class doesen't care, then who will? Anyway, my other idea was to have a function returning a c_bitmap with the file already loaded, but this doesen't make much sense to me, as that function would need access to pretty much everything inside the c_bitmap class. Is there something I'm missing here? Or I'm just overreacting about my initial problem? Thank you!

Share this post


Link to post
Share on other sites
In my oppinion haveing a bitmap class know how to load a bitmap makes perfectly good sense. It's a bitmap class it should know everything about bitmaps.

Quote:
Original post by algumacoisaqualquer
However, doesen't that breakes encapsulation? I mean, I keep thinking that my bitmap class shouldn't care about loading files, but if that class doesen't care, then who will?


one way to over come this is to make it so the class loads the bitmap from memory. So you would have a seperate resource manager that can take a file and load it into memory as raw data. then you pass that raw data to your bitmap class and it translates it to its form of bitmap image.

Share this post


Link to post
Share on other sites
I guess the OO way would be more like this:
// java like language
IStream stream = new FileStream("filename"); // MemoryStream, NetStream etc
ILoader loader = new ImageBmpLoader(stream); // or LoaderFactory.buildLoader(stream);
Image img = new loader.load(); // image can be a vector, or bitmap based, which in turn can be range based or float based and indexed or not or some other storage type.


and get rid of the "bool& success", throw an exception instead.

Share this post


Link to post
Share on other sites
Separate. The in-memory representation of an image should be independent from the on-disk representation of the file from which it was extracted. Ultimately, you want to have something like:


In-Memory Memory-Map

32-bit Bitmap -\ /- BMP format -\
| |- TGA format -| /- disk
24-bit Bitmap -| |- PNG format -| |
+--- loader ---+- JPEG format -+--- reader ---+- memory
8-bit Bitmap -| |- GIF format -| ^ |
| |- TIFF format -| | |- network
RLE Bitmap -/ \- SVG format -/ \--zip-/


I would consider:

img::bitmap
read_bitmap(const std::string & name, //< Read from this file
const img::format & fmt) //< Expect this image format
{
// Open the file in binary mode
std::fstream file(name, std::ios::binary);

// Create a loader for that file and format
img::loader input(file, fmt);

// Create a bitmap without initializing its contents, and set
// its internal representation mode to '24-bit'
img::bitmap out(img::no_init, img::bitmap::bpp24);

// Use the generic communication between "loader" and "bitmap",
// independent of their internal representation.
input >> out;

// Return the bitmap. This operation should be made cheap by
// using a COW implementation.
return out;
}

int main()
{
// Get a bitmap
img::bitmap image =
read_bitmap("foo.png", img::format::PNG);
}

Share this post


Link to post
Share on other sites
Don't throw an exception in the constructor. ;)

I generally use constructors purely to initialise the internal state of the object. I move any processing to an init() function or something similar. Constructors should always succeed.

If you really wanted to have the constructor do the loading, you can have a flag in the object to determine if the bmp is valid or not. So if the loading failed, you'd set mValid to false, and use some bmpObject.isValid() to test if the load went ok.

Share this post


Link to post
Share on other sites
Quote:
Original post by instinKt
Don't throw an exception in the constructor. ;)


Er, why?

Quote:
Constructors should always succeed.


Er, why? The actual C++ 'good practices' requirement is that A constructed object should always be in an usable state. This does not say anything about a constructor never failing—and constructors routinely fail when they cannot construct an usable object due to unexpected problems.

Quote:
If you really wanted to have the constructor do the loading, you can have a flag in the object to determine if the bmp is valid or not. So if the loading failed, you'd set mValid to false, and use some bmpObject.isValid() to test if the load went ok.


This is contrary to A constructed object should always be in an usable state. Here, you're making constructed objects potentially unusable (invalid) just so that constructors always succeed (something that few C++ programmers actually expect).

Make the class simpler in the cases where it works (no validity checking, no initialized-but-not-usable tidbits) and just throw an exception if you can't construct it.

Share this post


Link to post
Share on other sites
I agree with ToohrVyk - if a constructor can fail, then throw an exception -- don't rearrange the class so that it can't fail just for the sake of it (usually by creating a zombie object).

For this whole topic of how to use C++ "properly", I strongly recommended reading Scott Myers Effective C++ books. They're easy to read, make a lot of sense, and will get you jobs!

Share this post


Link to post
Share on other sites
Quote:
Original post by ToohrVyk
Quote:
Original post by instinKt
Don't throw an exception in the constructor. ;)


Er, why?

Quote:
Constructors should always succeed.


Er, why? The actual C++ 'good practices' requirement is that A constructed object should always be in an usable state. This does not say anything about a constructor never failing—and constructors routinely fail when they cannot construct an usable object due to unexpected problems.

Quote:
If you really wanted to have the constructor do the loading, you can have a flag in the object to determine if the bmp is valid or not. So if the loading failed, you'd set mValid to false, and use some bmpObject.isValid() to test if the load went ok.


This is contrary to A constructed object should always be in an usable state. Here, you're making constructed objects potentially unusable (invalid) just so that constructors always succeed (something that few C++ programmers actually expect).

Make the class simpler in the cases where it works (no validity checking, no initialized-but-not-usable tidbits) and just throw an exception if you can't construct it.


A long time ago, (I've been trying to remember when and exactly what I was working on, but I seem to have forgotten everything) I was working up some nice code with a heap of new classes that were all doing processing in the constructor and throwing exceptions if problems came up.

Now I don't remember if I was having some unexpected issues, or if someone peered over my shoulder and said "don't do that", referring to the exceptions in the constructor. It had something to do with the fact that the memory would be allocated, but your "handle" to the object would not be assigned, because the constructor did not complete and you would end up with memory holes.

I've done some reading now and see that it's perfectly legal to throw in a constructor, which has really made me curious as to why I was told/found out otherwise. I found a hint that it may have once been true?

Share this post


Link to post
Share on other sites
The destructor of an object doesn't get run if the constructor throws, but that's a reason to design the constructor properly - not a reason to avoid throwing from constructors.

Share this post


Link to post
Share on other sites
I personally like to do file-loading through factory methods like so:


class Bitmap
{
private:
Color* pixels;
public:
static Bitmap* FromFile(std::string& fileName);

Bitmap(int width, int height);
~Bitmap();
};


you'd then use it like this


Bitmap* img = Bitmap::FromFile("c:/images/puppy.bmp");

Share this post


Link to post
Share on other sites
Quote:
Original post by instinKt
Now I don't remember if I was having some unexpected issues, or if someone peered over my shoulder and said "don't do that", referring to the exceptions in the constructor. It had something to do with the fact that the memory would be allocated, but your "handle" to the object would not be assigned, because the constructor did not complete and you would end up with memory holes.

I've done some reading now and see that it's perfectly legal to throw in a constructor, which has really made me curious as to why I was told/found out otherwise. I found a hint that it may have once been true?


This is generally only a problem if you are using raw pointers and not using RAII in your code to ensure automagical clean up.

With RAII code and smart pointers if something fails in the constructor then you can throw an exception and everything will undo correctly without leaving 'holes' in memory.

Constructors always run the risk of failing for anything non-trivial anyway as any call to new runs the risk of throwing an exception (std::bad_alloc if memory serves), so unless you are going to avoid allocating memory in your constructor and say use an init function (generally a case of ewww!) you are always in danger of an exception occuring, which brings us back around to using RAII technics in code.

Share this post


Link to post
Share on other sites
Quote:
Original post by Nitage
The destructor of an object doesn't get run if the constructor throws, but that's a reason to design the constructor properly - not a reason to avoid throwing from constructors.


Yes but from what I remember the (supposed) issue was that not only would the object's destructor not be called, but the memory for the object itself would not get freed. So even if you cleaned up any memory allocation you did in the constructor upon a fail, the memory for the object itself would be lost.

Has this ever been true? Or was the person/article/whatever I heard it from, interpreting the problem wrongly?

Share this post


Link to post
Share on other sites
Quote:
Original post by instinKt
Quote:
Original post by Nitage
The destructor of an object doesn't get run if the constructor throws, but that's a reason to design the constructor properly - not a reason to avoid throwing from constructors.


Yes but from what I remember the (supposed) issue was that not only would the object's destructor not be called, but the memory for the object itself would not get freed. So even if you cleaned up any memory allocation you did in the constructor upon a fail, the memory for the object itself would be lost.

Has this ever been true? Or was the person/article/whatever I heard it from, interpreting the problem wrongly?


There could definitely have been pre-standard compilers with this behavior. However, the standard explicitly states what should happen to the memory:

Quote:
C++ Standard, 5.3.4 New, §17 and §18

If any part of the object initialization described above terminates
by throwing an exception and the new-expression does not contain a
new-placement, then the deallocation function
(_basic.stc.dynamic.deallocation_, _class.free_) is called to free the
memory in which the object was being constructed, after which the
exception continues to propagate in the context of the new-expression.

If any part of the object initialization described above terminates by
throwing an exception and the new-expression contains a new-placement,
if the type of the object being created is a class type, a name lookup
is performed on the name of operator delete using the same lookup
rules as those used to find operator new (_class.free_). Otherwise, a
name lookup is performed on the name of operator delete in the scope
of the new-expression (or in global scope for ::new ). If the lookup
succeeds and exactly one of the declarations found matches the decla-
ration of that placement operator new, then the matching placement
operator delete shall be called (_basic.stc.dynamic.deallocation_).
If no matching placement delete is found, propagating the exception
does not cause the object to be deleted.

Share this post


Link to post
Share on other sites
Well, thank you all for the answers! I don't have a very good idea for what I'm going to make right now (some non-related boring work that I have to do first is keeping me busy), but at least I have a better idea for what not to do. Actually, I'm thinking about making a class that just places the whole file in a char stream or something like that (I'll take a look if something like that doesn't already exists in the std), than another class takes that stream and converts into a actual bitmap. The OO part is probably on how I make that arrangement work, but that is going to have to wait.

Oh, and another thing: I considered using throws, since I really wanted to learn how to use them, but I heard that the palm compiler that I'm using doesn't support try/catch properly, so I decided to ignore them (well yes, not the brightest idea on the book but...)

Anyway, thank you all for the answers!

Share this post


Link to post
Share on other sites

This topic is 3725 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this