patterncppMinor
Engineering a minimalist image interface without templates
Viewed 0 times
withoutimageinterfacetemplatesengineeringminimalist
Problem
I'm practicing different ways of class design, so I'd like feedback on the design characteristics. Of course code practice comments are also welcome.
It was a conscious decision to omit templates. My next version will use a class template to store image data, similar to CImg and
Header:
```
#ifndef AIMG
#define AIMG
#include
#include
class AImg
{
public:
const std::vector dimsizes; //or std::array of length 4?
const int N; //# of elements ( = product of dimension sizes)
unsigned int Bytes {1}; //bytes per element
std::string tag; //meta info like "rgb"
std::string cast;//used for returning data of type cast
void* data; //pointer to some array/container/byte-blob
AImg(const std::vector&); //you have to specify image size when creating. But may be empty of data.
void setdata(void*);
void* getdata();
//example of pseudo-template method:
void plusone(std::string);
int operator()(unsigned int); //variable args would allow better indexing
private:
int getN();
//checkcast() ?
};
//public fcns
AImg::AImg(const std::vector& input): dimsizes(input), N(getN()), cast("") {}
int AImg::operator()(unsigned int idx){
if (cast=="int")
{return ((int*)data)[idx];} else return 0;
}
void AImg::setdata(void input){data=(void)input;}
void* AImg::getdata(){return data;}
void AImg::plusone(std::string cast){
if (cast=="int")
{
int idata = (int)data;
for (int i = 0;i<N;i++){idata[i] += 1;}
}
else if (cast == "float")
{
float fdata = (float)data;
for (int i = 0;i<N;i++){fdata[i] += 1.0;}
}
//...other types
// careful: N is based on elements of some user-defined type. Not all types have the same precision (#bytes).
// So you'd need to norma
It was a conscious decision to omit templates. My next version will use a class template to store image data, similar to CImg and
boost::GIL. I intend to make more classes with increasing complexity (templates, abstract base class, variadic templates). Until I reach a detailed yet highly abstract image class.Header:
```
#ifndef AIMG
#define AIMG
#include
#include
class AImg
{
public:
const std::vector dimsizes; //or std::array of length 4?
const int N; //# of elements ( = product of dimension sizes)
unsigned int Bytes {1}; //bytes per element
std::string tag; //meta info like "rgb"
std::string cast;//used for returning data of type cast
void* data; //pointer to some array/container/byte-blob
AImg(const std::vector&); //you have to specify image size when creating. But may be empty of data.
void setdata(void*);
void* getdata();
//example of pseudo-template method:
void plusone(std::string);
int operator()(unsigned int); //variable args would allow better indexing
private:
int getN();
//checkcast() ?
};
//public fcns
AImg::AImg(const std::vector& input): dimsizes(input), N(getN()), cast("") {}
int AImg::operator()(unsigned int idx){
if (cast=="int")
{return ((int*)data)[idx];} else return 0;
}
void AImg::setdata(void input){data=(void)input;}
void* AImg::getdata(){return data;}
void AImg::plusone(std::string cast){
if (cast=="int")
{
int idata = (int)data;
for (int i = 0;i<N;i++){idata[i] += 1;}
}
else if (cast == "float")
{
float fdata = (float)data;
for (int i = 0;i<N;i++){fdata[i] += 1.0;}
}
//...other types
// careful: N is based on elements of some user-defined type. Not all types have the same precision (#bytes).
// So you'd need to norma
Solution
A few notes about things and stuff (I know, my introductions are always original):
-
About that comment:
The standard library does have a function to compute the accumulated product of a range. It's called
Agreed, it's a bit verbose. But the function is quite generic since it may work with any well-formed binary operation. Note that if you are using C++14, you can use
-
Also, you mentioned
-
Your case isn't really consistent. For local variables, the generally accepted practice is to use
-
Ok, I hate to say this, but
-
Instead of that:
Array subcript tends to use the... well, the array subcript
-
I don't think that the method
-
About that comment:
//doesn't STL have a std::prod() ?!?!The standard library does have a function to compute the accumulated product of a range. It's called
std::accumulate. You can use it like this:int n = std::accumulate(
std::begin(dimsizes), std::end(dimsizes),
1, std::multiplies{}
);Agreed, it's a bit verbose. But the function is quite generic since it may work with any well-formed binary operation. Note that if you are using C++14, you can use
std::multiplies<> instead of std::multiplies and let the type be deduced.-
Also, you mentioned
std::array. If you know for sure that you will only store 4 values and not more, then please, by all means, use an std::array instead of an std::vector.-
Your case isn't really consistent. For local variables, the generally accepted practice is to use
snake_case everywhere and to drop every capital letter.-
Ok, I hate to say this, but
void AImg::plusone(std::string cast) without templates really hurts me. I know that you plan to change that later, but the current version still hurts me ç___ç Also, since you're not altering the std::string, it would be better to take a const std::string& parameter instead.-
Instead of that:
int AImg::operator()(unsigned int idx);Array subcript tends to use the... well, the array subcript
operator[] instead of the parenthesis. Unless you have to feed it several parameters, in which case operator() is sometimes preferred, but that's not your case.int AImg::operator[](unsigned int idx);-
I don't think that the method
setdata really needs a cast from void to void:void AImg::setdata(void* input) { data = input; }Code Snippets
//doesn't STL have a std::prod() ?!?!int n = std::accumulate(
std::begin(dimsizes), std::end(dimsizes),
1, std::multiplies<int>{}
);int AImg::operator()(unsigned int idx);int AImg::operator[](unsigned int idx);void AImg::setdata(void* input) { data = input; }Context
StackExchange Code Review Q#95233, answer score: 6
Revisions (0)
No revisions yet.