Determine number of even, odd, and zero digits of a given integer

Question

I have been programming in C++ for around a month now, just long enough, I figure, to develop bad habits. Can anyone point out where I'm making mistakes and offer topics I should investigate to understand why?

#include <iostream>
#include <cassert>
#include <sstream>
#include <string>
using namespace std;
string get_line();
int long get_integer();
int* flex(long int num);
void test_flex();
int main()
{ 
 //flex(get_integer());
 test_flex();
}
/* 
 * === FUNCTION =============================================================
 * Name: flex
 * Description: flex takes one integer argument and returns an then number
 * of odd, even and zero digits.
 * flex(111022) -> [3,2,1] i.e [odd,even,zero's]
 * ============================================================================
 */
 int*
flex (long int num)
{
 int res;
 int even = 0;
 int odd = 0;
 int zero = 0;
 while(num != 0){
 res = num % 10;
 if(res == 0){
 zero++;
 }
 else if(res % 2 == 0){
 even++;
 }
 else{
 odd++;
 }
 num = (num - (num % 10))/10;
 }
 int oez[3] = {odd,even,zero};
 return oez;
} /* ----- end of function flex ----- */
/* 
 * === FUNCTION =============================================================
 * Name: test_flex
 * Description: test suite for flex function
 * ============================================================================
 */
 void
test_flex ( )
{
 assert(flex(1)[0] == 1); 
 assert(flex(1)[1] == 0);
 assert(flex(1)[2] == 0);
 assert(flex(112220)[0] == 2);
 assert(flex(112220)[1] == 3);
 assert(flex(112220)[2] == 1);
} /* ----- end of function test_flex ----- */
/* 
 * === FUNCTION ======================================================================
 * Name: get_integer
 * Description: checks if input is integer
 * =====================================================================================
 */
 long int
get_integer ()
{
 while(true){
 stringstream converter;
 converter << get_line();
 int long result;
 if(converter >> result){
 char remaining;
 if(converter >> remaining)
 cout << "Unexpected character: " << remaining << endl;
 else{
 return result;
 }
 } else
 cout << "Please enter an integer." << endl;
 cout << "Retry:" << endl;
 }
} /* ----- end of function get_integer ----- */
/* 
 * === FUNCTION ======================================================================
 * Name: get_line
 * Description: wraps getline
 * =====================================================================================
 */
 string
get_line()
{
 string result;
 getline(cin,result);
 return result;
} /* ----- end of function get_line ----- */

Answer 1

Do not see the point in: get_line()

get_line(line);
// Vs
std::getline(std::cin, line);

Get Integer is well done. Most people forget to test if there is anything else remaining on the line.

A simple enhancement is:

 char remaining;
 if(converter >> remaining)
 // easier to rewrite as:
 if (!converter.str().empty())

Also there is already a boost function that does something similar:

long value = boost::lexical_cast<long>(line); // Will throw if anything left on line.

Over complicating this:

 num = (num - (num % 10))/10;

You can simplify it too:

 num /= 10; // Integer division truncates.

I think this is a logical error:

 if(res == 0){
 zero++;
 }
 else if(res % 2 == 0){ // zero is an even number
 // do you really want that else?

Your one major error is:

int oez[3] = {odd,even,zero};
return oez;

You are returning a pointer to an array that has gone out of scope. Personally I would return a std::vector<int>. Don't worry on a simple returning like this the copy back of the array will be optimized out (look up RVO and NVRO).

std::vector<int> oez = {odd,even,zero};
return oez;

Answer 2

I presume your code's brace format and function heading format is dictated by instructor and I won't comment on it. The code is presented clearly enough. One comment says problem 8, a couple of others say 10.

I deem it inappropriate to have a get_line routine; for code that would take 2 lines inline and be perfectly clear, you have about 13 lines of subroutine with no obvious reason for same.

The digit-counting code looks rather verbose to me, with tests like num != 0 and tests in awkward order. I'd rewrite the digit-counting code as follows.

 while (num) {
 res = num % 10;
 if (res)
 if (res & 1)
 ++odd;
 else
 ++even;
 else
 ++zero;
 num /= 10;
 }

• Drew Verlee: Yea, i don't know why i made get_line its own function. It's also now obvious to me that num - (num % 10), isn't doing anything. My instructor hasn't dictated anything. I'm bouncing between different styles i see in different texts and what i was comfortable with in python...