How to improve C++ Program?

[AlexCantor]

Following C++ program compiled and ran using MS VS2022 Community edition.

Please help improve this program (main, power function, #defines etc) using C++20 features.

In main, how to not use type casting?

	cout << "Message : " << (int)(msg) << endl;

In power function, can we improve if statement using c++20 feature?

// power prog
// power_pgm.cpp

#include <iostream>

using namespace std;

#define MAR 2

#define CONST_A 0xE0
#define CONST_B 0x00
#define CONST_C 0xA0

int power(const int es_in, unsigned char* msg_out)
{
	double es = 0.1 * es_in;
	double qe = 5.0;
	double no = qe + 1.0;
	double ma = no + MAR;

	if (es < no)
	{
		*msg_out = CONST_A;
	}
	else if (es < ma)
	{
		*msg_out = CONST_B;
	}
	else
	{
		*msg_out = CONST_C;
	}

	return(0);
}



int main()
{
	std::cout << "Hello Program." << std::endl;

	int es_in = 64;
	unsigned char msg = ' ';

	power(es_in, &msg);
	cout << "Message : " << (int)(msg) << endl;

	es_in = 41;
	msg = ' ';

	power(es_in, &msg);

	cout << "Message : " << (int)(msg) << endl;

	es_in = 90;
	msg = ' ';

	power(es_in, &msg);

	cout << "Message : " << (int)(msg) << endl;

	return(0);
}

Hello Program.
Message : 0
Message : 224
Message : 160

[JLBorges]

#include <iostream>

enum class power_message { A = 0xE0, B = 0x00, C = 0xA0 };

std::ostream& operator<< ( std::ostream& stm, power_message message )
{
    if( message == power_message::A ) return stm << int(message) << " (power_message::A)" ;
    else if( message == power_message::B ) return stm << int(message) << " (power_message::B)" ;
    else return stm << int(message) << " (power_message::C)" ;
}

power_message power( int es_in )
{
    static constexpr double MAR = 2 ;
	static constexpr double qe = 5.0;
	static constexpr double no = qe + 1.0;
	static constexpr double ma = no + MAR;

    const double es = 0.1 * es_in;
    if( es < no ) return power_message::A ;
    else if( es < ma ) return power_message::B ;
    else return power_message::C ;
}

int main()
{
    for( int es_in : { 64, 41, 90 } )
        std::cout << "es_in: " << es_in << ", power_message: " << power(es_in) << '\n' ;
}

https://coliru.stacked-crooked.com/a/a82a8f3c3b24fbae

[AlexCantor]

Thank you so much JLBorges,

For your excellent enhancements to the program.
Your updates to enum class, power function and main function are excellent and elegant.

Is it possible to enhance readability and understandability of (to a C programmer code reviewer) the operator<< overloading function, using say
named lamda expressions or some other C++20 feature please?

Thanks and best regards,

[JLBorges]

A C programmer code reviewer!

May be factor out the identification of the enumerator into a separate function,
in which a using-enum-declaration may make it look more familiar to a C programmer.
And provide copious text book / tutorial style comments, I suppose.

#include <iostream>

enum class power_message { A = 0xE0, B = 0x00, C = 0xA0 }; // scoped enumeration

char power_message_enum_char( power_message message ) // return the character (identifier) of the enumerator
{
    using enum power_message ; // introduce power_message's enumerator names into the current scope
    if( message == A ) return 'A' ; // A here refers to power_message::A
    else if( message == B ) return 'B' ; // power_message::B
    else return 'C' ; // not A or B, so in a well-written program it must be power_message::C
}

std::ostream& operator<< ( std::ostream& stm, power_message message )
{
    // there is no implicit conversion from a scoped enumerator to an integer,
    // so a cast is needed to retrieve its numeric value as an integer
    return stm << int(message) << " (power_message::" << power_message_enum_char(message) << ')' ;
}

power_message power( int es_in )
{
    static constexpr double MAR = 2 ;
	static constexpr double qe = 5.0;
	static constexpr double no = qe + 1.0;
	static constexpr double ma = no + MAR;

    const double es = 0.1 * es_in;
    if( es < no ) return power_message::A ;
    else if( es < ma ) return power_message::B ;
    else return power_message::C ;
}

int main()
{
    for( int es_in : { 64, 41, 90 } ) // for each integer es_in in the initializer_list
        std::cout << "es_in: " << es_in << ", power_message: " << power(es_in) << '\n' ;
}

[jonnin]

for something totally different in C or c++ any, really.. I left the comments in so you can see what happened as I reduced it. No need to return 0 if its always going to do that, make it void. Not sure the final index is fully
simplified, maybe it can be reduced a little more.

void power(const int es_in, unsigned char* msg_out)
{
	double es = 0.1 * es_in;
	//double qe = 5.0;
	//double no = 6.0; //qe + 1.0; ///6
	//double ma = 8.0; //no + MAR; ///8
	unsigned char res[]{CONST_C,CONST_A,CONST_B};
	//*msg_out = res[(es<no) + (!(es<no))*2*(es<ma)];
	*msg_out = res[(es<6.0) + (!(es<6.0))*2*(es<8.0)];
	//return(0);
}

or, after reduction, just
void power(const int es_in, unsigned char* msg_out)
{
	double es = 0.1 * es_in;
	unsigned char res[]{CONST_C,CONST_A,CONST_B};
	*msg_out = res[(es<6.0) + (!(es<6.0))*2*(es<8.0)];
}

[AlexCantor]

Hi JLBorges,
Thank you so much for making operator function much more elegant.

Best regards,


Thank you jonnin, for your great code improvement suggestions.

Best regards,

[AlexCantor]

Hi JLBorges,

I need the enum class member names to be kind of names shown in the program below:

Is the power_message_enum_char function shown below correct or any better way to write without type casting with char?

#include <iostream>

enum class power_message { ABC = 0xE0, PQR_ST = 0x00, XYZ = 0xA0 }; // scoped enumeration

char power_message_enum_char( power_message message ) // return the character (identifier) of the enumerator
{
    using enum power_message ; 
    if( message == power_message::ABC) return char(ABC) ;
    else if( message == power_message::PQR_ST) return char(PQR_ST) ;
    else return char(XYZ) ;
}

std::ostream& operator<< ( std::ostream& stm, power_message message )
{
    return stm << int(message) << " (power_message::" << power_message_enum_char(message) << ')' ;
}

power_message power( int es_in )
{
    static constexpr double MAR = 2 ;
	static constexpr double qe = 5.0;
	static constexpr double no = qe + 1.0;
	static constexpr double ma = no + MAR;

    const double es = 0.1 * es_in;
    if( es < no ) return power_message::ABC ;
    else if( es < ma ) return power_message::PQR_ST;
    else return power_message::XYZ;
}

int main()
{
    for( int es_in : { 64, 41, 90 } ) // for each integer es_in in the initializer_list
        std::cout << "es_in: " << es_in << ", power_message: " << power(es_in) << '\n' ;
}

Above program is giving below shown output:

es_in: 64, power_message: 0 (power_message::)
es_in: 41, power_message: 224 (power_message::α)
es_in: 90, power_message: 160 (power_message::á)

I prefer the output to be as shown below:

es_in: 64, power_message::ABC : 0 
es_in: 41, power_message::PQR::ST: 224 
es_in: 90, power_message::XYZ: 160 

Is it possible to show this kind of output?

[JLBorges]

> Is it possible to show this kind of output?

Yes; we can make the stream insertion operator do whatever we want it to do.

#include <iostream>
#include <string>

enum class power_message { ABC = 0xE0, PQR_ST = 0x00, XYZ = 0xA0 }; // scoped enumeration

// return the identifier (string) of the enumerator (with embedded _ replaced with ::)
std::string power_message_enum_string( power_message message )
{
    if( message == power_message::ABC) return "ABC" ;
    else if( message == power_message::PQR_ST) return "PQR::ST" ;
    else return "XYZ" ;
}

std::ostream& operator<< ( std::ostream& stm, power_message message )
{
    return stm << "power_message::" << power_message_enum_string(message) << " : " << int(message) ;
}

power_message power( int es_in )
{
    static constexpr double MAR = 2 ;
    static constexpr double qe = 5.0;
    static constexpr double no = qe + 1.0;
    static constexpr double ma = no + MAR;

    const double es = 0.1 * es_in;
    if( es < no ) return power_message::ABC ;
    else if( es < ma ) return power_message::PQR_ST;
    else return power_message::XYZ;
}

int main()
{
    for( int es_in : { 64, 41, 90 } ) // for each integer es_in in the initializer_list
        std::cout << "es_in: " << es_in << ", " << power(es_in) << '\n' ;
}

https://coliru.stacked-crooked.com/a/2b9e9d2f18b8a6ca

[AlexCantor]

Hi JLBorges,

Thank you so much for this most beautiful and elegant program.

Best regards,

[AlexCantor]

Is there an application (preferably freely downloadable) program that can take this C++ file as input and draw a flow chart of each of the functions in this cpp file please?

Also, it should draw graphical representation of a function called (from any given function) and called by functions.

These flow charts are to be understandable by higher managers.
These managers have excellent understanding of technical functionality and they want to review and ensure program's logic/algorithm is implemented correctly.

I have following program, Understand from Scitools:

https://www.scitools.com/

Right side of this web page shows the kind of graphs, this commercial software displays. We can click on the graphical image at the bottom right side of this web page, then we can see kind of graphs, this software displays.

But this is about $1000 per year license.
And it is not working with many C++20 features.


Is there a way to post image files (jpeg etc) to this forum, so that I can post a graphical representation of power function drawn/displayed (i.e. output) by Scitools software?

[jonnin]

not free, maybe a trial or older version, but understand c++ is really powerful and has a scripting like setup so you can make it do your own thing. Dunno if they have kept it going either, it wasn't super well received because its paid for.

[seeplus]

As an alternative to power_message_enum_string(), consider which is easier to extend to more enum items without having to add to the if-else ladder or using a switch().

std::string power_message_enum_string(power_message message) {
	using ET = std::underlying_type_t<power_message>;
	using enum power_message;
	static_assert(ET(std::min({ ABC, PQR_ST, XYZ })) >= 0);

	static const std::vector<std::string> names { [&] {
		std::vector<std::string> nam(static_cast<ET>(std::max({ ABC, PQR_ST, XYZ })) + 1, "Invalid");

		nam[static_cast<ET>(ABC)] = "ABC";
		nam[static_cast<ET>(PQR_ST)] = "PQR::ST";
		nam[static_cast<ET>(XYZ)] = "XYZ";
		return nam;
	} () };

	return ET(message) >= 0 && ET(message) < names.size() ? names[static_cast<ET>(message)] : "Invalid";
}

[JLBorges]

Use table lookup:

#include <iostream>
#include <string>
#include <map>

enum class power_message { ABC = 0xE0, PQR_ST = 0x00, XYZ = 0xA0 }; // scoped enumeration

// return the identifier (string) of the enumerator (with embedded _ replaced with ::)
std::string power_message_enum_string( power_message message )
{
    using enum power_message ; // C++20
    static const std::map< power_message, std::string > enumerator_map { {ABC,"ABC"}, {PQR_ST,"PQR::ST"}, {XYZ,"XYZ"} } ;

    if( const auto iter = enumerator_map.find(message) ; iter != enumerator_map.end() ) return iter->second ;
    else return "***invalid***" ;
}

std::ostream& operator<< ( std::ostream& stm, power_message message )
{ return stm << "power_message::" << power_message_enum_string(message) << " : " << int(message) ; }

int main()
{
    for( power_message msg : { power_message::ABC, power_message::PQR_ST, power_message::XYZ, power_message(1) } )
        std::cout << msg << '\n' ;
}

https://coliru.stacked-crooked.com/a/f8458d211dc96282

For something more elaborate, consider using a library. For example, magic_enum (MIT license, non-viral)
https://github.com/Neargye/magic_enum

[AlexCantor]

Hi JLBorges,
Thanks a bunch for adding the excellent Lookup map STL feature to this program.

I have added back the missing power function and previous main function as shown below:

#include <iostream>
#include <string>
#include <map>

enum class power_message { ABC = 0xE0, PQR_ST = 0x00, XYZ = 0xA0 }; // scoped enumeration

// return the identifier (string) of the enumerator (with embedded _ replaced with ::)
std::string power_message_enum_string(power_message message)
{
    using enum power_message; // C++20
    static const std::map< power_message, std::string > enumerator_map{ {ABC,"ABC"}, {PQR_ST,"PQR::ST"}, {XYZ,"XYZ"} };

    if (const auto iter = enumerator_map.find(message); iter != enumerator_map.end()) return iter->second;
    else return "***invalid***";
}

std::ostream& operator<< (std::ostream& stm, power_message message)
{
    return stm << "power_message::" << power_message_enum_string(message) << " : " << int(message);
}

power_message power(int es_in)
{
    static constexpr double MAR = 2;
    static constexpr double qe = 5.0;
    static constexpr double no = qe + 1.0;
    static constexpr double ma = no + MAR;

    const double es = 0.1 * es_in;
    if (es < no) return power_message::ABC;
    else if (es < ma) return power_message::PQR_ST;
    else return power_message::XYZ;
}

int main()
{
    for (int es_in : { 64, 41, 90 }) // for each integer es_in in the initializer_list
        std::cout << "es_in: " << es_in << ", " << power(es_in) << '\n';
}

How to modify for loop of this main function, to test "***invalid***" ? case of
power_message_enum_string function please?

Next I need to use exception handling features of C++20 to trap detailed errors.

Thanks and best regards,

[AlexCantor]

Thank you very much, seeplus:

For suggesting, power_message_enum_string function's alternate code.


Hello jonnin,

Many thanks for the information about Scitools Understand software.

Best regards,

[JLBorges]

> How to modify for loop of this main function, to test "***invalid***" ?
> case of power_message_enum_string function please?
> Next I need to use exception handling features of C++20 to trap detailed errors.

For this particular problem, none of this is either required or recommended. Just because there is a vector or a map or because there are exceptions, we do not have to use them in our code. It may, at worst, carelessly engender potential undefined behaviour as in the case of the vector code that was posted earlier or at best, make the code less readable by introducing needless complications.

If this is a classroom exercise, and the teacher demands that these must be used in this program, may be something like this:

#include <iostream>
#include <string>
#include <map>
#include <stdexcept>

enum class power_message { ABC = 0xE0, PQR_ST = 0x00, XYZ = 0xA0 }; // scoped enumeration

static const std::map< power_message, std::string > enumerator_map
{ {power_message::ABC,"ABC"}, {power_message::PQR_ST,"PQR::ST"}, {power_message::XYZ,"XYZ"} };

power_message checked( power_message message ) // return unchanged if valid, else throw
{
    if ( const auto iter = enumerator_map.find(message); iter != enumerator_map.end() ) return message ;
    else throw std::domain_error( "bad value for power_message" ) ;
}

// return the identifier (string) of the enumerator (with embedded _ replaced with ::)
std::string power_message_enum_string( power_message message )
{ return enumerator_map.find( checked(message) )->second ; }

std::ostream& operator<< (std::ostream& stm, power_message message)
{ return stm << "power_message::" << power_message_enum_string(message) << " : " << int(message); }

power_message power( int es_in )
{
    #ifndef NDEBUG
        // in debug build, provide a facility to simulate an invalid power_message error
        // return an invalid power message for negative values of es_in
        // note: the underlying type of the enum is not fixed. So to avoid undefined behaviour,
        //       we need to return an invalid value that within the range of the enumeration [ 0x00, 0xFF ]
        if( es_in < 0 ) return power_message(1) ;
    #endif // NDEBUG

        static constexpr double MAR = 2;
        static constexpr double qe = 5.0;
        static constexpr double no = qe + 1.0;
        static constexpr double ma = no + MAR;

        const double es = 0.1 * es_in;
        if (es < no) return power_message::ABC;
        else if (es < ma) return power_message::PQR_ST;
        else return power_message::XYZ;
}

int main()
{
    for (int es_in : { 64, 41, 90, -9 }) // for each integer es_in in the initializer_list
    {
        std::cout << "es_in: " << es_in << ", " ;

        try { std::cout << power(es_in) << '\n'; }
        catch( const std::exception& ex ) { std::cerr << "***error***: " << ex.what() << '\n'  ; }
    }
}

https://coliru.stacked-crooked.com/a/826ef44527c7bf73

[seeplus]

carelessly engender potential undefined behaviour as in the case of the vector code

yeah - now fixed above. This is why you have code reviews and proper testing...

[JLBorges]

It is still not fixed.

For an enumeration whose underlying type is fixed, the values of the enumeration are the values of the underlying type. Otherwise, the values of the enumeration are the values representable by a hypothetical integer type with minimal width M such that all enumerators can be represented. The width of the smallest bit-field large enough to hold all the values of the enumeration type is M. It is possible to define an enumeration that has values not defined by any of its enumerators. If the enumerator-list is empty, the values of the enumeration are as if the enumeration had a single enumerator with value 0.
https://eel.is/c++draft/enum#dcl.enum-8

[AlexCantor]

Because of the not availability of the other libraries being used in our application program, my manager do not want to use C++20 features yet.

Manager only wants C++17 features to be used.

Is there a way to make changes to this program using C++ 17 features?

Thanks,

#include <iostream>
#include <string>
#include <map>
#include <stdexcept>

enum class power_message { ABC = 0xE0, PQR_ST = 0x00, XYZ = 0xA0 }; // scoped enumeration

static const std::map< power_message, std::string > enumerator_map
{ {power_message::ABC,"ABC"}, {power_message::PQR_ST,"PQR::ST"}, {power_message::XYZ,"XYZ"} };

power_message checked( power_message message ) // return unchanged if valid, else throw
{
    if ( const auto iter = enumerator_map.find(message); iter != enumerator_map.end() ) return message ;
    else throw std::domain_error( "bad value for power_message" ) ;
}

// return the identifier (string) of the enumerator (with embedded _ replaced with ::)
std::string power_message_enum_string( power_message message )
{ return enumerator_map.find( checked(message) )->second ; }

std::ostream& operator<< (std::ostream& stm, power_message message)
{ return stm << "power_message::" << power_message_enum_string(message) << " : " << int(message); }

power_message power( int es_in )
{
    #ifndef NDEBUG
        // in debug build, provide a facility to simulate an invalid power_message error
        // return an invalid power message for negative values of es_in
        // note: the underlying type of the enum is not fixed. So to avoid undefined behaviour,
        //       we need to return an invalid value that within the range of the enumeration [ 0x00, 0xFF ]
        if( es_in < 0 ) return power_message(1) ;
    #endif // NDEBUG

        static constexpr double MAR = 2;
        static constexpr double qe = 5.0;
        static constexpr double no = qe + 1.0;
        static constexpr double ma = no + MAR;

        const double es = 0.1 * es_in;
        if (es < no) return power_message::ABC;
        else if (es < ma) return power_message::PQR_ST;
        else return power_message::XYZ;
}

int main()
{
    for (int es_in : { 64, 41, 90, -9 }) // for each integer es_in in the initializer_list
    {
        std::cout << "es_in: " << es_in << ", " ;

        try { std::cout << power(es_in) << '\n'; }
        catch( const std::exception& ex ) { std::cerr << "***error***: " << ex.what() << '\n'  ; }
    }
}

[keskiverto]

First:

Please help improve this program (main, power function, #defines etc) using C++20 features.

Then:

Manager only wants C++17 features to be used.

Is there a way to make changes to this program using C++ 17 features?

Why are you moving the target?