You are here:

C++/Triangle types C programming question

Advertisement


Question
This program is supposed to prompt the user for three lengths which make up a triangle and print the appropriate message. it is also supposed to ask if the user wants to try another triangle(Y or N) and repeat as many times as the user wishes. My problems are: 1)The program enters a fail state when tested with decimal values(instead of int) and fails to execute the loop properly when the user enters Y for yes or N for no. What I am doing wrong?..Thanks!
//interactive program using three length values to determine appropriate type of triangle
#include<iostream>
#include<iomanip>
#include<fstream>
#include<cstring>
#include<cmath>
using namespace std;

void TriangleType(int&,int&, int&);
enum Triangles{EQUILATERAL, ISOSCELES, SCALENE};

int main()
{
   int sideA;
   int sideB;
   int sideC;
  Triangles trianType;
  char choice ;


  TriangleType(sideA, sideB, sideC);

  {
     if(sideA+sideB<sideC||sideB+sideC<sideA||sideA+sideC<sideB)
        cout<<sideA<<','<<sideB<<','<<sideC<<" is"<<" NOT A TRIANGLE"<<endl;
       else if(sideA==sideB&&sideB==sideC)
        trianType=EQUILATERAL;
      else if(sideA==sideB||sideB==sideC||sideA==sideC)
        trianType=ISOSCELES;
     else
        trianType=SCALENE;   
     
  }
   switch(trianType)
  {
  case EQUILATERAL:cout<<"A triangle with sides "<<sideA<<','<<sideB<<','<<sideC<<" is an"<<" EQUILATERAL"<<endl;
     break;
  case ISOSCELES:cout<<"A triangle with sides "<<sideA<<','<<sideB<<','<<sideC<<" is an"<<" ISOSCELES"<<endl;
     break;
  case SCALENE:cout<<"A triangle with sides "<<sideA<<','<<sideB<<','<<sideC<<" is a"<<" SCALENE"<<endl;
      break;
  }
  cout<<"Would you lie to try another triangle?(enter Y for yes and N for no)"<<endl;
  cin>>choice;
  while(choice)
  {
     if(choice=='Y'||choice=='y')
     TriangleType(sideA, sideB, sideC);      
  }
  return 0;

}



  void TriangleType(/*out*/int& lengthA,
         /*out*/int& lengthB,
         /*out*/int& lengthC)

  {
     bool invalidData;
     invalidData=true;
     while(invalidData)
     {      
     
       cout<<"Enter the length of the first side of the triangle"<<endl;
        cin>>lengthA;
        {
        if(lengthA>0)
        invalidData=false;
        else
        cout<<"Warning: entry invalid, length must be an integer"<<endl;
       
        }
     }
       

     invalidData=true;
     while(invalidData)
     {
         cout<<"Enter the length of the second side of the triangle"<<endl;
        cin>>lengthB;
       {
       
       if(lengthB>0)
       invalidData=false;
       else
       cout<<"Warning: entry invalid, length must be an integer"<<endl;
         
       }

       
     }
       invalidData=true;
     while(invalidData)
     {
  
         cout<<"Enter the length of the third side of the triangle"<<endl;
        cin>>lengthC;
        {
       
         if(lengthC>0)
         invalidData=false;
         else          
         cout<<"Warning: entry invalid, length must be an integer"<<endl;
        }

       
     }


  }  

Answer
Note: there is a 10000 CHARACTER limit on answers. The more questions are embedded in your single posted question the less space I can devote to each.

1/ I assume by decimal numbers you mean real numbers which are approximated using floating point numbers in C++. Of course it fails. You are asking the program to interpret data formatted as floating point numbers as integer values. You should test for stream failures on cin and then clear the relevant state flags and eat and throw away any remaining un used characters thus:

   cout << "Enter the length of the first side of the triangle"
        << endl;
   cin >> lengthA;

   if ( cin.good() && lengthA > 0 )
   {
       invalidData = false;
   }
   else if ( cin.bad() )
   {
       throw std::ios_base::failure("std::cin : Stream bad.");
   }
   else if ( cin.eof() )
   {
       throw std::ios_base::failure("std::cin : Unexpected EOF.");
   }
   else // negative, zero or badly formatted integer
   {
       cout << "Length must be a positive non zero integer" << endl;

   // Clear only the fail state flag if it is set
   // and ignore any remaining characters on the line
         cin.clear( cin.rdstate() & ~std::ios::failbit);
         cin.ignore( numeric_limits<streamsize>::max(), '\n' );
   }

You will have to include ios and limits. I have chosen to throw exceptions if cin has a hard error (is bad) or the end of file is reached. This implies you should try this block of code and catch and handle the exceptions somewhere. I have trimmed some of your text only so its fits on one line in my editor! I also added braces around all if else clauses. The reason can be seen in the expanded final else clause - your single statement is now three and so required the braces to be added anyway so you might as well put them in to start with, then you will not forget and wonder why new code is being executed at wrong times. You can decide for yourself whether this is a good idea or not.

The interesting part is that the final else clause which covers your original error of an integer greater than zero plus any stream formatting failures. To handle the latter I have used a form of the stream clear operation that clears all flags and sets those it is passed. The complex parameter passed to this operation is the result of all the currently set stream state flags as returned by cin.rdstate() less the fail flag, which is turned off by bitwise ANDing the set flags value with the ones complement of the fail flag bit value. Finally all additional characters on the input line are ignored, ensuring there is nothing remaining to be read and so the user will be asked for data.

This code will prevent your code from crashing or endlessly looping but has a problem. The stream failure is only detected for the input _after_ the one where the bad value was entered. This is because the part of the floating point number before the decimal point is a valid integer and so gets interpreted as such. Thus entering 1.23 for lengthA will set lengthA to 1 and then cause an immediate failure for lengthB followed by a request for lengthB to be re-entered. On the other hand if instead you entered non-numeric input, maybe the word "two" or something, then the failure is detected immediately. Resolving this problem is not going to be neat.

Here are some possible remedies:

- Read double floating point values and check that they have no fractional part in addition to being non-zero, positive and not too large, then assign them to the (unsigned) integers you actually want. You should keep error checking code similar to that above for bad streams, end of file and other badly formatted values (such as "two point three").

- As the previous suggestion but convert all your code to use floating point values.

- Read each complete line of user input into a string and use a string stream to parse it into a number. You can then check the string stream for errors as above (in addition to cin) and after a good value is parsed to length(A, B or C) try to extract more data to see if that was all, if it was not then it may have been a rubbish value.


2/ Do you understand how the test in the while loop below works?

   cin>>choice;
   while(choice)
   {
   // ...
   }

If the expression is true then the while loop continues. For C and C++ true is _any_ non zero value. Or more precisely any value that is convertible to bool and for integer types zero converts to false and all other values to true. So your while loop could be equivalently expressed thus:

   while ( choice!=0 )
   {
   // ...
   }

The only time I can see choice being zero is if the read from cin failed and it happens to have been initialised to a zero value, which is not all that likely as local stack objects are _not_ initialised to any preset value but take on whatever value is in the memory they are allocated. That is the first problem - your loop will most likely spin for ever and if you entered 'y' or 'Y' then it will repeatedly ask for three length values but _not_ do anything with them. You whole function needs to be in a loop which is terminated if the user says no. The loop is probably best bottom tested - a do ... while loop:

   int sideA;
   int sideB;
   int sideC;
   Triangles trianType;
   char choice('N');
   do
   {
       TriangleType(sideA, sideB, sideC);

   // ...

       do
       {
        cout << "Would you like to try another triangle?"
         " (enter Y for yes and N for no)"
         << endl;
        cin >> choice;
       }
       while (   choice!= 'N' && choice!= 'n'  
         && choice!= 'Y' && choice!= 'y'
         );

     }
     while ( 'Y'==choice || 'y'==choice );

It seems to me that your code is badly structured.

First the name TriangleType for the function that obtains data from the user is very misleading. I think this may have been part of your problem with the yes / no loop.

Second the code that reads the values is repeated. Why not write a function called something like GetLengthFromUser that returns a single length value and takes the prompt displayed to the user.

Third I would re-designate TriangleType to take the three side lengths and return a Triangles enumeration.

Forth I would consider the error handling. If you keep the exceptions I threw in the example cin error handling code then add try..catch blocks to main. I would add a NOT_A_TRIANGLE enumeration value to Triangles and return this from the revised TriangleType function if you detect the lengths cannot form a triangle.

Finally, do not be afraid of spacing out your code and laying it out pleasantly. Code is for people to read. Make it as easy as possible for them (which may well be you) to understand.

The revised outline for main would look something like:

#include <iostream>
#include <ios>
#include <string>
#include <limits>

using namespace std;

enum Triangles{NOT_A_TRIANGLE, EQUILATERAL, ISOSCELES, SCALENE};

// Display prompt to user and accept and return a
// positive integer result
int GetLengthFromUser( std::string const & prompt );

// Return the triangle type given three size lengths
Triangles TriangleType(int sideA, int sideB, int sideC);

int main()
{
   char choice('N');
   try
   {
       do
       {
       // Get values from the user
         int sideA
         ( GetLengthFromUser
         ("Enter the length of the 1st side of the triangle"
         )
         );
         int sideB
         ( GetLengthFromUser
         ("Enter the length of the 2nd side of the triangle"
         )
         );
         int sideC
         ( GetLengthFromUser
         ("Enter the length of the 3rd side of the triangle"
         )
         );

       // Display common part of result text
         cout  << "A triangle with sides "
         << sideA << ',' << sideB << ',' << sideC << " is";

      // Display varying part of result text based on enum value
      // returned from TriangleType
         switch ( TriangleType(sideA, sideB, sideC) )
         {
         case EQUILATERAL:
         cout << " an" << " EQUILATERAL" << endl;
         break;
         case ISOSCELES:
         cout << " is an"<<" ISOSCELES" << endl;
         break;
         case SCALENE:
         cout << " is a"<<" SCALENE" << endl;
         break;
         case NOT_A_TRIANGLE :
         cout << " is"<<" NOT A TRIANGLE" << endl;
         break;
         }

        // Ask user to enter Y, y, N or n
         do
         {
         cout  << "Would you like to try another triangle?"
         " (enter Y for yes and N for no)"
         << endl;
         cin >> choice;
         }
         while (   choice!= 'N' && choice!= 'n'  
         && choice!= 'Y' && choice!= 'y'
         );

       // If user answered Y or y go around again
       }
       while ( 'Y'==choice || 'y'==choice );
   }
   catch ( std::ios_base::failure & e )
   {
       cerr << "FATAL ERROR: " << e.what() << endl;
   }
}

The function GetLengthFromUser would be the result of the code I showed for the answer to sub-question 1, suitably wrapped up and modified to include any remedies for the late detection of floating point numbers.

The function TriangleType is as described above: It is the block of code in your original code in { and } in main (which makes me think you pasted from elsewhere or something), with the cout statement for not a triangle replaced with an assignment of the new NOT_A_TRIANGLE enum value to trianType and the addition of return trianType; at the end of the function.

I hope this has been of help to you. Have fun.  

C++

All Answers


Answers by Expert:


Ask Experts

Volunteer


Ralph McArdell

Expertise

I am a software developer with more than 15 years C++ experience and over 25 years experience developing a wide variety of applications for Windows NT/2000/XP, UNIX, Linux and other platforms. I can help with basic to advanced C++, C (although I do not write just-C much if at all these days so maybe ask in the C section about purely C matters), software development and many platform specific and system development problems.

Experience

My career started in the mid 1980s working as a batch process operator for the now defunct Inner London Education Authority, working on Prime mini computers. I then moved into the role of Programmer / Analyst, also on the Primes, then into technical support and finally into the micro computing section, using a variety of 16 and 8 bit machines. Following the demise of the ILEA I worked for a small company, now gone, called Hodos. I worked on a part task train simulator using C and the Intel DVI (Digital Video Interactive) - the hardware based predecessor to Indeo. Other projects included a CGI based train simulator (different goals to the first), and various other projects in C and Visual Basic (er, version 1 that is). When Hodos went into receivership I went freelance and finally managed to start working in C++. I initially had contracts working on train simulators (surprise) and multimedia - I worked on many of the Dorling Kindersley CD-ROM titles and wrote the screensaver games for the Wallace and Gromit Cracking Animator CD. My more recent contracts have been more traditionally IT based, working predominately in C++ on MS Windows NT, 2000. XP, Linux and UN*X. These projects have had wide ranging additional skill sets including system analysis and design, databases and SQL in various guises, C#, client server and remoting, cross porting applications between platforms and various client development processes. I have an interest in the development of the C++ core language and libraries and try to keep up with at least some of the papers on the ISO C++ Standard Committee site at http://www.open-std.org/jtc1/sc22/wg21/.

Education/Credentials

©2016 About.com. All rights reserved.