Advice/opinions on usage of c++

[Kanoisa]

Heya

Thanks for taking the time to look at this, i would like some people to review my curent project which is written in c++ using visual studio 2008. Although in VS08 it does not use anything that makes it dependant on it (i dont think) i just wanted to be sure that visual studio liked my code before the move to a graphics application. (project files attached in zip file)

What im trying to do
Make a dos personal organiser application that will allow me to test the functionality of my classes and helper functions to ensure they work before making a graphical application.

what it should do and contain
years (C_Year) this is the top level class which is slightly more than a container class it will hold all the days of the year some data about the year (which year? is it a leap year?) member functions such as initilise and (not implimented yet)add/remove task to day, display day(s) tasks.

The code handles leap years in calculating the first day of the specified year and has a internal flag to know if the current year is indeed a leap year.

The init procedure in the year is as far as i have in the year's functionality it calculates the first day of the year and initilises all the days with the correct date day of the week and month.

Days (C_Day) are fully working classes they hold the information about the day they represent such as day of the week date month and a list of tasks for that day. C_Day has functions to add a task to the day which includes use of a protected sorting function. If more than one task is added the double linked list of tasks is sorted from earliest occouring in the day to latest occouring. there is also a function provided to print the task information to the console.

Tasks (C_Task) this class holds the information about a task such as its name, the task's details, scheduled time and schedule type, schedule type is currently only held in the task and has no effect yet but i hope to impliment that later as some tasks may happen only once, on a daily basis meonthly basis etc ,..

in some of my helper functions i have overloaded them to accept either a u16 (unsigned short) or an E_DAY (enumerated type of days of the week) i have chosen not to use a template class as i want to ensure that those are the only two input types people can use with the helper functions to prevent bugs. It was a design decision i took however if anyone can suggest a good argument for template classes or how it could actually prevent bugs better than my overloaded fucntions id be happy to hear about it.

what id like from this
I dont think theres much more i can say here other than what id like in a response. I would love if you could make any comments on my use of classes, c++ in general etc which could help me improve my coding practise as im a soley self taught programmer (from books but no teacher) and of course any suggestions / improvements on what im doing or how im doing it that would improve the application would be ooh so much appreciated also.

[Narue]

You should really reconsider relying on non-portable behavior (eg. fflush(stdin)) and non-portable functions (eg. gets_s) if you can avoid it. I'd also recommend merging your months into an array. Even if you waste a few elements of an array, it's still a win in complexity if you consider nothing more than the tedious C_Year::InitYear. Beyond that I'd just be nitpicking.

One thing I did wonder about while reading your code: why virtual destructors? You have no other virtual member functions, and unless you know for a fact that the types will be treated polymorphically down the road, why add the complexity?

[Kanoisa]

Is there a good alternative to the fflush(stdin)? and i could use normal gets i guess it was really to shut up visual studio warnings. As for the single months array would you suggest something like C_Day[366] and have all days in one array like that? if so it wont watse any space as i over allocated one day for the case of a leap year anyway. in terms of complexity i find this solution to be relatively simple as my access functions (which ive done since the last post) such as C_Year::AddTask(E_MONTH month ,u8 date) i check the date to be in range of the month then simply switch the month and then pass the date as the array index, as an example

case(JAN):
m_jan[date].AddTask();

I guess to do it as one array i would have to add all the days of each month untill the month i want to the index to access it? i dont quite see how i could make it simpler with just one array if you could explian that maybe?

Finally for virtual destructors i was sticking to the coding style in my c++ book in which the author suggests its good practise to make destructors virtual so when the event comes where polymorphism is used its already handled.

Appreciate the time you took to offer a response and it makes me glad to see there are only a few issues with how im coding ill make the changes and continue to work.

Again thanks :)

[Narue]

Is there a good alternative to the fflush(stdin)?

Yes, several. I've already posted a sticky on this forum concerning flushing input streams, and the web is full of other resources, so I'm not inclined to repeat what's already been said.

i could use normal gets

No, you should forget gets even exists. This is C++ anyway, and you'd be better served by std::getline and std::string. Then buffer overflow safety becomes a moot point.

As for the single months array would you suggest something like C_Day[366] and have all days in one array like that?

I was thinking more along the lines of C_Day array[12][31] , but it's just a suggestion. Take it with a grain of salt, but take care not to rub that salt in your wounds when you find yourself writing twelve sequential loops instead of two nested loops every time you want to do something with all of the months in a year.

Finally for virtual destructors i was sticking to the coding style in my c++ book in which the author suggests its good practise to make destructors virtual so when the event comes where polymorphism is used its already handled.

That's stupid. It's very simple:

Are you planning to use those classes as polymorphic bases?
(Y) - Okay, make the destructor virtual. Also, best practice is to also make the base an abstract class.
(B) - Okay, don't bother. Why add something that's not immediately useful or likely to be useful in the future?

[Kanoisa]

Well i guess that just wraps it all up thanks, ill find the input stream flushing thread and have a go at implimenting something similar or stealing your code:P and thanks about the destructers thing ill keep that in mind :)