Thread: Please check my C++

  1. #241
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    So what is the purpose of found and the return value. In general, I prefer my menu items to be self-contained, so my way to do the ShowFleetList() would probably not have any parameters at all [except perhaps the fleet itself as a const reference, in the case where there may be more than one fleet - example if you have a call-center covering the Acity and Beetown rental places, and you want to be able to see if there's a Ford Focus available in either place - but if we accept the limitation that there is only one location covered by this database [or the location is part of the database, instead of having multiple databases - large rental companies often borrow cars from one location to another - once when I was in Texas and had a Maryland registered car from Hertz for example]].

    To your concrete request for an example, I don't know how to solve the whole problem, because I don't know what the whole context of the call is.

    Perhaps something like this:
    Code:
    void MenuListFleet(menucontext &ctxt)
    {
       FleetIterator iterate;
       bool found;
       bool ret;
       ret = ShowFleetList(iterate=fleet.begin(), found);
       ... // What I don't understand is what the purpose of found and ret is. Also, iterate seems a bit un-needed here. 
    }
    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  2. #242
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by matsp View Post
    So what is the purpose of found and the return value. In general, I prefer my menu items to be self-contained, so my way to do the ShowFleetList() would probably not have any parameters at all [except perhaps the fleet itself as a const reference, in the case where there may be more than one fleet - example if you have a call-center covering the Acity and Beetown rental places, and you want to be able to see if there's a Ford Focus available in either place - but if we accept the limitation that there is only one location covered by this database [or the location is part of the database, instead of having multiple databases - large rental companies often borrow cars from one location to another - once when I was in Texas and had a Maryland registered car from Hertz for example]].

    To your concrete request for an example, I don't know how to solve the whole problem, because I don't know what the whole context of the call is.

    Perhaps something like this:
    Code:
    void MenuListFleet(menucontext &ctxt)
    {
       FleetIterator iterate;
       bool found;
       bool ret;
       ret = ShowFleetList(iterate=fleet.begin(), found);
       ... // What I don't understand is what the purpose of found and ret is. Also, iterate seems a bit un-needed here. 
    }
    --
    Mats
    1. [ret] Sorry, the function doesn't return bool... It's rather void...
    2. [found] I used the function ShowFleetList to display a list of available cars or a single car if we used search. I use bool to decide what to display
    3. I could have called it this way..
    Code:
    ShowFleetList(fleet.begin(), found);
    but still have parameters...

    inside ShowFleetList [iter is received as parameter. Also, searched as bool]
    Code:
    	// Display one one record if fleet was searched
    	if (iter!=fleet.end() && searched){
    		iter->Write(cout, true);
    	}
    	else
    	{
    		// Display fleet details 
    		for(iter=fleet.begin(); iter!=fleet.end(); iter++){
    			iter->Write(cout, true);
    		}
    	}
    iter is only used for searched fleet, hence is re-initialized for displaying entire list... But then, maybe i can refix this not to have parameters, but i you suggesting that because of menucontex (or table am planning to use) all my functions should not have parameters or return values?

  3. #243
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by csonx_p View Post
    1. [ret] Sorry, the function doesn't return bool... It's rather void...
    2. [found] I used the function ShowFleetList to display a list of available cars or a single car if we used search. I use bool to decide what to display
    3. I could have called it this way..
    Code:
    ShowFleetList(fleet.begin(), found);
    but still have parameters...

    inside ShowFleetList [iter is received as parameter, searched as bool]
    Code:
    	// Display one one record if fleet was searched
    	if (iter!=fleet.end() && searched){
    		iter->Write(cout, true);
    	}
    	else
    	{
    		// Display fleet details 
    		for(iter=fleet.begin(); iter!=fleet.end(); iter++){
    			iter->Write(cout, true);
    		}
    	}
    iter is only used for searched fleet, hence is re-initialized for displaying entire list... But then, maybe i can refix this not to have parameters, but i you suggesting that because of menucontex (or table am planning to use) all my functions should not have parameters or return values?
    Isn't most of what ShowFleetList does completely different for showing the search result vs. listing the whole fleet anyways?

    So if you are in "search", then just call iter->Write(cout, true) as well as doing the find operation [if iter != fleet.end(), that is] in the search function itself. That way, ShowFleetList() don't need a bool, and you are setting iter to fleet.begin(), so it's no point in passing that as a parameter either.

    Of course, in the event that you want to do more work inside "display one car", you may want to have a separate function that displays one car, and call that function with the iter parameter both at successful find and in the ShowFleetList().

    There is no point in having a function when it does "if (x) something; else someother" when x is always a constant when calling - you should just call something or someother directly.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  4. #244
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by matsp View Post
    Isn't most of what ShowFleetList does completely different for showing the search result vs. listing the whole fleet anyways?

    So if you are in "search", then just call iter->Write(cout, true) as well as doing the find operation [if iter != fleet.end(), that is] in the search function itself. That way, ShowFleetList() don't need a bool, and you are setting iter to fleet.begin(), so it's no point in passing that as a parameter either.

    Of course, in the event that you want to do more work inside "display one car", you may want to have a separate function that displays one car, and call that function with the iter parameter both at successful find and in the ShowFleetList().

    There is no point in having a function when it does "if (x) something; else someother" when x is always a constant when calling - you should just call something or someother directly.

    --
    Mats
    You didn't answer my other concerns though! Are you suggesting i should try to change all my functions not to have parameters & returns? If not, given function : bool foo(string a, int b);, how would i pass this to addmenu? or how should menucontext handle this?

  5. #245
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    All menu functions need to have the same calls [at least if you want to use a table-based menu system].

    My solution was to use a context to pass arguments.

    In your example of "bool foo(string a, int b)", where does a and b come from, and what does the return value represent. My feeling is that your current menu partly also does the functionality that should be in the actual menu-entry. So if a and b are inputs from the user and the bool is some success indicator, then the menufunction itself would be something like this:
    Code:
    void MenuFoo(menucontext &ctxt)
    {
       string answer;
       cout << "Some question?";
       cin >> answer;
       cout << "Question two?";
       int number;
       cin >> number;
       if (foo(answer, number))
           cout >> "That went OK" << endl;
       else
           cout >> "Didn't work" << endl;
    }
    But again, this is not a set in stone solution - there are many different solutions to this particular problem, and you are the one designing the application and who knows how you want to solve certain problems.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  6. #246
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    I dunno what your original question was, but I'm pretty sure it should have been fixed by now (240+ replies).
    Consider starting new threads with new specific questions.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  7. #247
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by Salem View Post
    I dunno what your original question was, but I'm pretty sure it should have been fixed by now (240+ replies).
    Consider starting new threads with new specific questions.
    Yes and no. This is actually an evolving thread making progress, rather than the usual "it's so far from the original thread content that no one can remember what it was about".

    But sure, it could just as well be split into sections. I sometimes find it easier with one long thread, because the original concept is still there, but of course alot of content all in one thread.. At the Planet Catfish forum, we have one long-running thread with 424 posts in it, and it's still active and useful.

    But it may of course help others a bit more if the different parts where discussed in different threads.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  8. #248
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by Salem View Post
    I dunno what your original question was, but I'm pretty sure it should have been fixed by now (240+ replies).
    Consider starting new threads with new specific questions.
    You will notice that at times i do start new threads which may be more a generic question like yesterday [http://cboard.cprogramming.com/showthread.php?t=105496] which is not highly tightened to this specific thread. Mats and Elysia has contributed so much into this thread and still are. Mats understand what i'm doing and his helped made this threads a success to my project.

  9. #249
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Code:
    Menu m;
    
    m.addMenu("Display List of cars", '3', ShowFleetList);
    
    //: error C3867: 'FleetMenu::ShowFleetList': function call missing argument list; use '&FleetMenu::ShowFleetList' to create a pointer to member
    Mats, in your example you not using any referencing

    Code:
    	m.addmenu("First menu", '1', first);
    	m.addmenu("QUit",       'q', quit);

  10. #250
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    I'm not, because my functions are free functions, not class functions - the compiler is correct in it's suggestion for remedy. Also note that the functions you use for this should be static.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  11. #251
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by matsp View Post
    I'm not, because my functions are free functions, not class functions - the compiler is correct in it's suggestion for remedy. Also note that the functions you use for this should be static.

    --
    Mats
    Now have a new strange problem... Anything suspicious about this class

    Code:
    // FleetMenu.h
    
    class FleetMenu
    {
    public:
    
    	friend class ContractMenu;
    
    	FleetMenu();
    	void RunFleet();
    
    	// Menu functions
    	static void Exit(MenuContext &ctxt);
    	static void AddNewCar(MenuContext &ctxt);
    	static void DeleteCar(MenuContext &ctxt);
    	static void ShowFleetList(MenuContext &ctxt);
    	static void CreateContract(MenuContext &ctxt);
    	static void UpdateFleetFile(MenuContext &ctxt);
    	static void searchCarDetails(MenuContext &ctxt);
    	static void TitleDescription(MenuContext &ctxt);
    	static void ReadFleetFromFile(MenuContext &ctxt);
    	static void EditFleetContract(MenuContext &ctxt);
    
    private:
    
    	typedef vector<Car> CarFleet;
    	typedef CarFleet::iterator FleetIterator;
    
    private:
    
    	void SortAlpha();
    	static void Pause();
    	static void Clrscr();
    	void ProgramLimits();
    	void SetReturnDates(Date& dates);
    	double amountDue(int days, int rate);
    	Car& matchRightCar(FleetIterator iter);
    	void ShowExistingContract(string contractID);
    	FleetIterator find(FleetIterator&, FleetIterator&, CAR_MAKE);
    
    private:
    	CarFleet fleet; 
    };
    When i try to use it's members from it's source file [FleeMenu.cpp] i get errors. i.e..

    Code:
    #include "FleetMenu.h"
    
    1.
    void FleetMenu::TitleDescription(MenuContext &ctxt)
    {
    
    	ProgramLimits();  // : error C2352: 'FleetMenu::ProgramLimits' : illegal call of non-static member function
    }
    
    2.
    fleet.push_back(car);  // : error C2228: left of '.push_back' must have class/struct/union
    It's as if FleetMenu class is not visible to FleetMenu.cpp

  12. #252
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    So is fleet inside the fleet menu your one and only fleet vector?

    I think you can solve the problem by putting a FleetMenu pointer in the menucontext, and using that object to call the member classes.

    pedantic lint-picking:
    Code:
    void SetReturnDates(Date& dates);
    Surely the return date of one car is one date? Also, whould this class not need to take a Car object?

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  13. #253
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by matsp View Post
    So is fleet inside the fleet menu your one and only fleet vector?
    Yes, that declaration you seeing is the one used and the only one...

    I think you can solve the problem by putting a FleetMenu pointer in the menucontext, and using that object to call the member classes.
    Code:
    // Class MenuContext
    class MenuContext 
    {
    public:
    	bool fquit;
    	
    	FleetMenu *menuPrt;  // <--- HERE
    
    	MenuContext() : fquit(false) {}
    	~MenuContext() {};
    };
    typedef void (*MenuFunc)(MenuContext &ctxt);
    How do i use it then? like this...

    Code:
    void FleetMenu::ReadFleetFromFile(MenuContext &ctxt)
    {
    ctxt->fleet.push_back(car);
    ctxt->ProgramLimits();
    }
    pedantic lint-picking:
    Code:
    void SetReturnDates(Date& dates);
    Surely the return date of one car is one date? Also, whould this class not need to take a Car object?
    sorry, that function is now obsolete... just deleted it (didn't pick it up after adding Date class)

    --
    Mats[/QUOTE]

  14. #254
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Code:
    void FleetMenu::ReadFleetFromFile(MenuContext &ctxt)
    {
    ctxt->MenuPrt->fleet.push_back(car);
    ctxt->MenuPrt->ProgramLimits();
    }
    You also would have to set the MenuPrt (did you mean Ptr, or Prt?) to a FleetMenu somewhere early in your code - perhaps in the context constructor.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  15. #255
    Registered User
    Join Date
    Apr 2008
    Posts
    610
    Quote Originally Posted by matsp View Post
    Code:
    void FleetMenu::ReadFleetFromFile(MenuContext &ctxt)
    {
    ctxt->MenuPrt->fleet.push_back(car);
    ctxt->MenuPrt->ProgramLimits();
    }
    You also would have to set the MenuPrt (did you mean Ptr, or Prt?) to a FleetMenu somewhere early in your code - perhaps in the context constructor.

    --
    Mats
    Mhh... I lost you, set it to/with what exactly... is this using the 'new' operator ?
    Code:
    FleetMenu *menuPtr = new FleetMenu();

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. BN_CLICKED, change button style
    By bennyandthejets in forum Windows Programming
    Replies: 13
    Last Post: 07-05-2010, 11:42 PM
  2. how to check input is decimal or not?
    By kalamram in forum C Programming
    Replies: 3
    Last Post: 08-31-2007, 07:07 PM
  3. Please check this loop
    By Daesom in forum C++ Programming
    Replies: 13
    Last Post: 11-02-2006, 01:52 AM
  4. A way to check for Win98 or WinXP
    By Shadow in forum A Brief History of Cprogramming.com
    Replies: 5
    Last Post: 10-31-2002, 11:06 AM
  5. how to check for end of line in a text file
    By anooj123 in forum C++ Programming
    Replies: 6
    Last Post: 10-24-2002, 11:21 PM