[lnkForumImage]
TotalShareware - Download Free Software

Confronta i prezzi di migliaia di prodotti.
Asp Forum
 Home | Login | Register | Search 


 

Forums >

comp.lang.c++

Help, looking for right tempalte/class declaration...

sccr13plyr

12/8/2008 4:10:00 PM

Hello,

I do not know if this forum is the right place to post this message.
If it is not, please let me know a better place to get my question
addressed. I am an old C programmer that became a self-taught C++
programmer. So, the following may simply be my ignorance of the C++
language.

I am at a site that has an older application written in C that has
over time been migrated to C++. This is a high performance
application that processes a 100 million records a day that are then
broken down into component transactions. We have a set of algorithms
that process these transactions and have a lot of duplicate logic.
We want to add functionality, simplify the code, and keep the
performance.

I prototyped out the following:

#include <stdio.h> // streams can be discussed another time... ;)

class BaseAlgorithm
{
public: virtual ~BaseAlgorithm (){}

public: inline void Display (const int id, const char* output)
{
(void) printf ("Algorithm: %d - %s\n", id, output);
}
};

template<int id, bool checkPrice, bool specificCode, bool currTran>
class GenericAlgorithm : public BaseAlgorithm
{
private: static const int algorithmId_ = id;
private: static const bool checkPriceChanged_ = checkPrice;
private: static const bool specificCode_ = specificCode;
private: static const bool currentTransOnly_ = currTran;

public: GenericAlgorithm (){}
public: virtual ~GenericAlgorithm (){}
public: inline void GenericLogic (int* data, int* env, char*
retInfo)
{
if (data == NULL)
if (env == NULL)
if (retInfo == NULL)
{
//
}

if (checkPriceChanged_)
{
if (specificCode_)
{
if (currentTransOnly_)
{
char v1[1000][1000];
v1[999][999] = 'd';
if (v1[999][999] == 'd')
{
}

Display (algorithmId_, "Check Price,
Specific Code, Current Tran");
}
else
{
Display (algorithmId_, "Check Price,
Specific Code, All Trans");
}
}
else
{
if (currentTransOnly_)
{
Display (algorithmId_, "Check Price, All
Code, Current Tran");
}
else
{
Display (algorithmId_, "Check Price, All
Code, All Tran");
}
}
}
else
{
if (specificCode_)
{
if (currentTransOnly_)
{
Display (algorithmId_, "Ignore Price,
Specific Code, Current Tran");
}
else
{
Display (algorithmId_, "Ignore Price,
Specific Code, All Trans");
}
}
else
{
if (currentTransOnly_)
{
Display (algorithmId_, "Ignore Price, All
Code, Current Tran");
}
else
{
Display (algorithmId_, "Ignore Price, All
Code, All Trans");
}
}
}
}
};

#include <stdlib.h>

int main (void)
{
GenericAlgorithm<159, true, true, true> test;
test.GenericLogic (NULL, NULL, NULL);

GenericAlgorithm<246, true, false, false> *oldTest = new
GenericAlgorithm<246, true, false, false>;
oldTest->GenericLogic (NULL, NULL, NULL);

return EXIT_SUCCESS;
}

This approach would collapse the three algorithms into one body of
logic in GenericLogic. By using constants, my desire is to have the
compiler create two separate versions of GenericLogic (one for each of
the new class declarations in main), removing code (and comparisons)
that would never need to be used.

Should I expect that the compiler is actually removing unreachable
code? Would this be compiler specific or I should be utilizing a
different C++ language construct? Also, the pointer declaration and
initialization becomes a little bulky, something I can do there?

Thanks in advance for your assistance, and sorry if this is not the
right venue...

sccr13plyr
2 Answers

Paavo Helde

12/8/2008 7:02:00 PM

0

sccr13plyr@gmail.com kirjutas:

> Hello,
>
> I do not know if this forum is the right place to post this message.
> If it is not, please let me know a better place to get my question
> addressed. I am an old C programmer that became a self-taught C++
> programmer. So, the following may simply be my ignorance of the C++
> language.
>
> I am at a site that has an older application written in C that has
> over time been migrated to C++. This is a high performance
> application that processes a 100 million records a day that are then
> broken down into component transactions. We have a set of algorithms
> that process these transactions and have a lot of duplicate logic.
> We want to add functionality, simplify the code, and keep the
> performance.
>
> I prototyped out the following:
>
> #include <stdio.h> // streams can be discussed another time... ;)
>
> class BaseAlgorithm
> {
> public: virtual ~BaseAlgorithm (){}

Virtual destructor might be needed if you allocate objects dynamically.
However, dynamic allocation is relatively slow, if you do this a lot, it
might affect performance.

>
> public: inline void Display (const int id, const char* output)

No need to repeat "public:" here. Also "inline" is implicit for in-class
defined member functions, no need to write it out here, and anyway
compilers decide by themselves nowadays what to inline.

> {
> (void) printf ("Algorithm: %d - %s\n", id, output);

(void) is not needed.

> }
> };
>
> template<int id, bool checkPrice, bool specificCode, bool currTran>
> class GenericAlgorithm : public BaseAlgorithm
> {
> private: static const int algorithmId_ = id;
> private: static const bool checkPriceChanged_ = checkPrice;
> private: static const bool specificCode_ = specificCode;
> private: static const bool currentTransOnly_ = currTran;

These static members are not needed, what's wrong with using template
parameters directly? Also, no need to repeat "private" before each
declaration.

>
> public: GenericAlgorithm (){}

Such a constructor is not needed, C++ makes the default ctor
automatically when needed.

> public: virtual ~GenericAlgorithm (){}
> public: inline void GenericLogic (int* data, int* env, char*
> retInfo)

"inline" superfluous again

> {
> if (data == NULL)
> if (env == NULL)
> if (retInfo == NULL)
> {
> //
> }
>
> if (checkPriceChanged_)
> {
> if (specificCode_)
> {
> if (currentTransOnly_)
> {
> char v1[1000][1000];
> v1[999][999] = 'd';
> if (v1[999][999] == 'd')
> {
> }
>
> Display (algorithmId_, "Check Price,
> Specific Code, Current Tran");
> }
> else
> {
> Display (algorithmId_, "Check Price,
> Specific Code, All Trans");
> }
> }
> else
> {
> if (currentTransOnly_)
> {
> Display (algorithmId_, "Check Price, All
> Code, Current Tran");
> }
> else
> {
> Display (algorithmId_, "Check Price, All
> Code, All Tran");
> }
> }
> }
> else
> {
> if (specificCode_)
> {
> if (currentTransOnly_)
> {
> Display (algorithmId_, "Ignore Price,
> Specific Code, Current Tran");
> }
> else
> {
> Display (algorithmId_, "Ignore Price,
> Specific Code, All Trans");
> }
> }
> else
> {
> if (currentTransOnly_)
> {
> Display (algorithmId_, "Ignore Price, All
> Code, Current Tran");
> }
> else
> {
> Display (algorithmId_, "Ignore Price, All
> Code, All Trans");
> }
> }
> }
> }
> };
>
> #include <stdlib.h>
>
> int main (void)

"void" not needed

> {
> GenericAlgorithm<159, true, true, true> test;
> test.GenericLogic (NULL, NULL, NULL);
>
> GenericAlgorithm<246, true, false, false> *oldTest = new
> GenericAlgorithm<246, true, false, false>;
> oldTest->GenericLogic (NULL, NULL, NULL);

Memory leak here, *oldTest is not released.

>
> return EXIT_SUCCESS;

nitpick: the above line is also not needed ;-)

> }
>
> This approach would collapse the three algorithms into one body of
> logic in GenericLogic. By using constants, my desire is to have the
> compiler create two separate versions of GenericLogic (one for each of
> the new class declarations in main), removing code (and comparisons)
> that would never need to be used.

Using constants certainly does not guarantee this, even if some compiler
happens to generate different code. In principle I think it should be
easier for the compiler to optimize out the branches if there were no
static variables, as the task would be simpler then.

>
> Should I expect that the compiler is actually removing unreachable
> code? Would this be compiler specific or I should be utilizing a
> different C++ language construct?

If the branch taking speed appears to be important, then you can apply
(maybe partial) template specialization. This can become quite verbose
though. Or maybe a policy-based design suits your needs better, look it
up.

> Also, the pointer declaration and
> initialization becomes a little bulky, something I can do there?

typedefs?

In general, though, if you are doing this not for education and just for
speeding the program up, then you should start from profiling your app
and finding out the bottlenecks. I would be quite surprised if branching
on a boolean parameter would show up in the results. Such branching might
affect something only in a very tight loop, and in this case the
branching should just be lifted out of the loop, no template trickery
needed.

hth
Paavo


Michael DOUBEZ

12/9/2008 9:28:00 AM

0

sccr13plyr@gmail.com a écrit :
> [snip]
> I am at a site that has an older application written in C that has
> over time been migrated to C++. This is a high performance
> application that processes a 100 million records a day that are then
> broken down into component transactions. We have a set of algorithms
> that process these transactions and have a lot of duplicate logic.
> We want to add functionality, simplify the code, and keep the
> performance.

After reading the code, you might consider refactoring quite a bit.
I would advice you to read (if not already done):
* Refactoring: Improving the Design of Existing Code
It will show you how to move around pieces of code
to fit and improve your design (what you are actually doing)
* Modern C++ Design Generic Programming and Design Patterns Applied
While a bit outdated concerning Loki, it will show you how
to use policies to achieve flexibility and simplification
for high performances.


> I prototyped out the following:
>
> #include <stdio.h> // streams can be discussed another time... ;)

Yes but it is however not the proper header:
#include <cstdio>

>
> class BaseAlgorithm
> {
> public: virtual ~BaseAlgorithm (){}
>
> public: inline void Display (const int id, const char* output)
> {
> (void) printf ("Algorithm: %d - %s\n", id, output);
> }
> };

Do you really need polymorhism on algorithm ? I would suggest you to
keep the algorithms orthogonal and make a type-erasure if you really
need polymorphic behavior.

I suspect that you want to do it to display data which is
counter-intuitive. Displaying data is usually part of the result of the
algorithm.

You can use a policy that manage result of algorithm:

//class to display algorithm information as a result of the algorithm
struct ResultDisplay
{
static void result(const int id, const char* result)
{
printf ("Algorithm: %d - %s\n", id, output);
}
};

> template<int id, bool checkPrice, bool specificCode, bool currTran>

Using booleans if fine from a performance point of view, the compiler is
likely to optimize them out in GenericLogic but they do not carry the
intent of what you are doing and you will have to add other booleans for
other case.

In addition to that the logic might be difficult to debug. Here, I have
used PricingPolicy that indicate if the price is checked or not and a
SelectTransactionPolicy that indicate which transaction to select.

template
<int id
,class PricingPolicy
,class SelectTransactionPolicy
,class ResultPolicy = ResultDisplay
>

> class GenericAlgorithm : public BaseAlgorithm
> {
> private: static const int algorithmId_ = id;
> private: static const bool checkPriceChanged_ = checkPrice;
> private: static const bool specificCode_ = specificCode;
> private: static const bool currentTransOnly_ = currTran;

This is needed only if you are likely to use them in another template
construct. If not, don't do it; it just add maintaining and clutter.

>
> public: GenericAlgorithm (){}
> public: virtual ~GenericAlgorithm (){}
> public: inline void GenericLogic (int* data, int* env, char*
> retInfo)
> {
> if (data == NULL)
> if (env == NULL)
> if (retInfo == NULL)

if( data == NULL || env == NULL || retInfo == NULL )

> {
> //
> }
>

Replace the following with orthogonal policies if possible.
An example with unique policy (Note: I don't know what specificCode and
currTran represent and how they interact or how to integrate them as
policy):

//Pricing policy
struct PriceNotChecked
{
static const char* check(const char* price)
{
return "Ignore Price";
}
};

struct PriceChecked
{
static const char* check(/* data */)
{
return "Check Price";
}
};


> [snip] long code

ResultPolicy::result(id,PricePolicy::check());

> }
> };
>
> #include <stdlib.h>
>
> int main (void)
> {
> GenericAlgorithm<159, true, true, true> test;

GenericAlgorithm<159,PriceChecked,AllTransaction> test;

> test.GenericLogic (NULL, NULL, NULL);
>
> GenericAlgorithm<246, true, false, false> *oldTest = new
> GenericAlgorithm<246, true, false, false>;

GenericAlgorithm<246,PriceChecked,CurrentTransaction> oldtest;

> oldTest->GenericLogic (NULL, NULL, NULL);
>
> return EXIT_SUCCESS;
> }
>
> This approach would collapse the three algorithms into one body of
> logic in GenericLogic. By using constants, my desire is to have the
> compiler create two separate versions of GenericLogic (one for each of
> the new class declarations in main), removing code (and comparisons)
> that would never need to be used.
>
> Should I expect that the compiler is actually removing unreachable
> code?

Yes, but this code must be correct. I don't think you will have this
problem here.

> Would this be compiler specific or I should be utilizing a
> different C++ language construct?

I would expect main vendor compiler to do so depending on their option.
But checking does not hurt.

> Also, the pointer declaration and
> initialization becomes a little bulky, something I can do there?

Use a typedef.

If you need partial specialization, you can do the following (it will be
simplier under c++09):

template<int id>
struct OldAlgorithm
{
typedef GenericAlgorithm<id,PriceChecked,CurrentTransaction> type;

//helper to avoid writing new OldAlgorithm<id>::type(...)
static type* create(void* param1, void* param2, void* param3)
{
return new type(param1,param2,param3);
}
};

And then:

OldAlgorithm<246>::type* oldtest=
OldAlgorithm<246>::create(NULL,NULL,NULL);

--
Michael