[OliveOyl3471]

Here's my latest assignment, and the instructor gave me a 10 out of 10 for it. But she did not provide any feedback as to how it could have been done better, or why the instructions said to do it differently than what I did...or how it could have been done according to the instructions.

So I'm asking you. Why should this program use an enum? It does not make sense to me, since we have to assign a value to each period. Is there a different/better way of using it than what I have done here? And would it be more efficient to use more functions as the instructions say:
//create a function that determines the period corresponding to a date
//create another function that returns the string corresponding to
//each identifier for the enum
//use a for loop to output the series of periods in the range

Also, did I get my calculations right, on the dates? It's B.C. so it's kinda backward.

Did I put the enum in the right spot? I'd prefer it not to be global, but could not get it to work otherwise.

Yes, I know you should not use system("Pause") but that is what all of my instructors have preferred, so far. At least I'm finally getting the hang of not using so many global variables, and learning how to clean up my main()

If there's anything else you see that could/should have been done better, please tell me! ♥

//Chapter 10 Homework
//Problem 3, Page 517, Textbook
//Due Date: 19/Apr

#include <iostream>

using namespace std;

    enum dates
    {
          Quaternary = int(2.5),
          Tertiary = 65,
          Cretaceous = 136,
          Jurassic = 192,
          Triassic = 225,
          Permian = 280,
          Carboniferous = 345,
          Devonian = 395,
          Silurian = 435,
          Ordovician = 500,
          Cambrian = 570,
          Precambrian = 4500
    };

    //get beginning year
    int getBegYr(int begYr)
    {
        cout<<"\nEnter a range of prehistoric dates (in millions of years), and I\n";
        cout<<"will tell you which periods are included within that range.\n\n";

        cout<<"Enter beginning year: ";
        cin>>begYr;

        return begYr;
    }

    //get ending year
    int getEndYr(int endYr)
    {
        cout<<"Enter ending year: ";
        cin>>endYr;

        return endYr;
    }

    bool errorCheck(int begYr, int endYr)
    {
    //make sure begYr > endYr because the dates are B.C.
        if(begYr<=endYr)
        {
            cout<<"The ending year must be less than the beginning year,\n\n";
            return true;
        }
        return false;
    }

    //determine & print results
    void printPeriods(int begYr, int endYr)
    {
      cout<<"\nPeriods included within that range: \n";
        if(endYr <= Quaternary)
        {
            cout<<"Quaternary\n";
        }
        if(endYr <= Tertiary && begYr > Quaternary)
        {
            cout<<"Tertiary\n";
        }
        if(endYr <= Cretaceous && begYr > Tertiary)
        {
            cout<<"Cretaceous\n";
        }
        if(endYr <= Jurassic && begYr > Cretaceous)
        {
            cout<<"Jurassic\n";
        }
        if(endYr <= Triassic && begYr > Jurassic)
        {
            cout<<"Triassic\n";
        }
        if(endYr <= Permian && begYr > Triassic)
        {
            cout<<"Permian\n";
        }
        if(endYr <= Carboniferous && begYr > Permian)
        {
            cout<<"Carboniferous\n";
        }
        if(endYr <= Devonian && begYr > Carboniferous)
        {
            cout<<"Devonian\n";
        }
        if(endYr <= Silurian && begYr > Devonian)
        {
            cout<<"Silurian\n";
        }
        if(endYr <= Ordovician && begYr > Silurian)
        {
            cout<<"Ordovician\n";
        }
        if(endYr <= Cambrian && begYr > Ordovician)
        {
            cout<<"Cambrian\n";
        }
        if(begYr >= Precambrian)
        {
            cout<<"Precambrian\n";
        }
    }

int main ()
{
    char quit = 'Y';
    while(quit != 'N')
    {
        bool flag = true;
        int begYr = 0, endYr = 0;
        while (flag)
        {
        begYr = getBegYr(begYr);
        endYr = getEndYr(endYr);
        flag = errorCheck(begYr, endYr);
        }

        //confirm input data
        cout<<begYr<<" to " <<endYr<<endl;

        printPeriods(begYr, endYr);

        cout<<"\nWould you like to continue? Y/N: ";
        cin>>quit;
        quit = toupper(quit);
    }
system("pause");
return 0;
}

This post has been edited by OliveOyl3471: 18 April 2009 - 09:40 AM

[PsychoCoder]

Hey Olive I'm going to move this to the C++ Homework forum

[janotte]

I'll just deal with this bit.

OliveOyl3471, on 18 Apr, 2009 - 08:14 AM, said:

And would it be more efficient to use more functions as the instructions say:
//create a function that determines the period corresponding to a date
//create another function that returns the string corresponding to
//each identifier for the enum
//use a for loop to output the series of periods in the range

Better is the word you are probably looking for here rather than "efficient".

The instructor has a design in mind that uses an enum. Because an enum allows access to the values using their index values it is a nice match for the incrementing counter in a for() loop. Allows for neat processing in the same kind of way that you increment through an array.

Breaking your program down into a collection of small functions is the preferred way to go in all cases. The smaller the function the easier it is to debug and to conduct unit tests that prove each function is working correctly. Each function then becomes a cog that has it's specific job to do and you slot the cogs together to build a program.

The other advantage is that once you have a function that you know works to do specific thing you can drop it into any program that needs it. As you program more you will hold a snippet file that has heaps of useful functions you have written and trust that you can paste into programs with a few mouse clicks.

In modern programming style main() is little more than a series of calls to functions perhaps wrapped in a loop or two.

The point of this way of thinking will be even clearer as you get further into OOP.

[OliveOyl3471]

Thanks Janotte. That makes sense.

I just couldn't figure out how to do what the instructions said.

Psycho, thanks. I thought it might belong over here, after I already had it in the other forum.

Any other suggestions/answers would be appreciated.

This post has been edited by OliveOyl3471: 18 April 2009 - 06:35 PM

[OliveOyl3471]

OliveOyl3471, on 18 Apr, 2009 - 11:14 AM, said:

Why should this program use an enum? It does not make sense to me, since we have to assign a value to each period.

Is there a different/better way of using it than what I have done here?

Also, did I get my calculations right, on the dates?

Did I put the enum in the right spot?

If there's anything else you see that could/should have been done better, please tell me! ♥

It's bad to quote yourself, I know. It's bad to 'bump' your own thread too.
But I'm ready for the flamethrowers, just as long as you also tell me what you think of my program.

[baavgai]

To be honest, I'm not sure what the enum gets you.

It's a way to enforce consistency within a program, but there kind of has to be a point to the enforcement. That is, some reason the user must be locked down. I can't really see where it applies here.

Here's a stab I took at it:

#include <iostream>

using namespace std;

enum PeriodType {
	Quaternary,Tertiary,Cretaceous,Jurassic,
	Triassic,Permian,Carboniferous,Devonian,
	Silurian,Ordovician,Cambrian,Precambrian,
	PeriodTypeCount
};

struct PeriodTypeRec {
	string name;
	int startYear;
} PeriodTypeValues[PeriodTypeCount] = {
	{"Quaternary",2},{"Tertiary",65},{"Cretaceous",136},{"Jurassic",192},
	{"Triassic",225},{"Permian",280},{"Carboniferous",345},{"Devonian",395},
	{"Silurian",435},{"Ordovician",500},{"Cambrian",570},{"Precambrian",4500}
};


PeriodType getPeriodForYear(int year) {
	int i=PeriodTypeCount;
	while(year<PeriodTypeValues[--i].startYear && i>0);
	return (PeriodType)i;
}

string &getPeriodName(PeriodType p) {
	static string oops("NA");
	if (p<0 || p>=PeriodTypeCount) { return oops; }
	return PeriodTypeValues[p].name;
}

void printPeriods(int begYr, int endYr) {
	cout << "Range:" << begYr << "BC to "<< endYr << "BC " << endl;
	cout << "Periods included within that range: " << endl;
	int j = getPeriodForYear(begYr);
	for(int i=getPeriodForYear(endYr); i<=j; i++) {
		cout<< getPeriodName((PeriodType)i) << endl;
	}
}

I'm not real fond of globals, either. In a sense, enum is global by it's nature, though. I'd consider wrapping the functions in a class, add a validation method, and bugger the enum.