views:

65

answers:

4

I have three classes, TImageProcessingEngine, TImage and TProcessing

TImageProcessingEngine is the one which i am using to expose all my methods to the world.

TImage is the one i plan to use generic image read and image write functions.

TProcessing contains methods that will perform imaging operations.

class TImageProcessingEngine
{
    public:
        TImage* mpImageProcessingEngine;

};

class TImage
{
    public:
        int ReadImage();
        int WriteImage();

    private:
            //a two dimensional array holding the pixel values
        tImageMatrix*  mpImageMatrix;
};

class TProcessing
{
    public:
        int ConvertToBinary();
        int ConvertToGrayScale();
};

My question is how do i access the object mpImageMatrix in class TProcessing? So that my calling application can use the following

TImageProcessingEngine* vEngine = new TImageProcessingEngine;

//Converts an input gray scsale image to binary image
vEngine->ReadImage().ConvertToBinary();

//Write the converted image to disk
vEngine->WriteImage();

delete vEngine;
vEngine = NULL;

//During this whole processing internally, 
//the image is read in to `mpImageMatrix` 
//and will also be holding the binarised image data, 
//till writing the image to disk.

Or Do you recommend any other approach to my class design?

A: 

First off, based on your provided code there are no ReadImage() & WriteImage() functions in the TImageProcessingEngine class, so the later code where you use such functionality is flawed.

As for the solution, you can make a getter function for the tImageMatrix pointer like this:

tImageMatrix* GetImageMatrix() { return mpImageMatrix; }

Then just pass that pointer (or a pointer to the whole TImage instance) to the TProcessing function you want to call.

Strom
Raj
A: 

You could create an accessor TImage class:

byte * pixelAt(unsigned x, unsigned y);
tibur
@tibur, Don't you think, this will be time costly, as you are trying to access an individual pixel?
Raj
If this function is inlined, you get no overhead.
tibur
+1  A: 

Why you want to have a separate TProcessing process, when it specifically has functions just accessing mpImageMatrix;

In OOP, you have to bind the data members and it's operations..

So, IMO, remove your TProcessing class and have both the functions within TImage..

Your TImage will be like,

class TImage
{
public:
    int ReadImage();
    int WriteImage();
    int ConvertToBinary();
    int ConvertToGrayScale();

private:
        //a two dimensional array holding the pixel values
    tImageMatrix*  mpImageMatrix;
};
liaK
Some people prefer to have class that has only 1 purpose. Therefore having a seperate Image processing class, next to a Image class which just is an image.
PoweRoy
@PoweRoy, Yeah, sounds valid for me... Thanx... :)
liaK
+1 @liaK, This is what i have implemented in the first place. But i wanted to move away from this. Thanks for your contribution.
Raj
+1  A: 

I would certainly recommend a different implementation, but let's check the design first.

I don't really understand the added value of TImageProcessingEngine, it doesn't bring any functionality.

My advice would be quite simple in fact:

  • Image class, to hold the values
  • Processing class (interface), to apply operations
  • Encoder and Decoder classes (interfaces), to read and write to different formats

It does make sense for the Processing class to have access to the images internal only if you can get efficiency from it (which is likely), in this case you can simply makes Processing friend and having it unpack the values for its derived

class Image
{
public:
  Image();

  void Accept(Processing& p);
  void Encode(Encoder& e) const; // Image is not modified by encoding

  void Decode(Decoder& d); // This actually resets the image content

private:
  friend class Processing;

  size_t mHeight;
  size_t mWidth;
  std::vector<Pixel> mPixels; // 2D array of Pixels
};

class Processing
{
public:
  void apply(Image& image)
  {
    this->applyImpl(image.mHeight, image.mWidth, image.mPixels);
  }

private:
  virtual void applyImpl(size_t h, size_t w, std::vector<Pixel>& pixels) = 0;
};

Encoder and Decoder follow the same principle.

Note how I never needed an explicit pointer, and the guaranteed correctness that results from it.

Matthieu M.
@Matthieu M - Your reply is more than what i was expecting. Thank you. Can you please explain how you declare this object Pixel in the following statement? std::vector<Pixel> mPixels; // 2D array of PixelsThanks
Raj
@Raj: well, what is a `Pixel` for you ? You said you had a matrix of pixels so I assumed you would know how to represent them. Typical representations include RGB or YCMV etc...
Matthieu M.
@Matthieu M- ho Ok thanks
Raj