Cmd-- 1.11

[BruceJohnJennerLawso]

@Stormhawk, I feel they have a point, as I don't plan on going solo forever. I've decided on the Horstmann style, which I think should be an acceptable format to most other coders, and the easiest for me to retrofit my previous code to.

But I wouldnt mind if someone gave actual feedback on the library itself...

[Dy7SLyTq]

is it multi platform? if not whats it for? im on linux which is why i ask

[Superdude]

is it a library. or .hpp, or .h files? I don't want to link to yet another library. :P

EDIT: I found out that it is .h files. Thanks. Is there any documentation on how to use it? I am kinda clueless about it.

EDIT2: I saw in your original post about no documentation. Can you at least give a working code example of how to use most of the features?

[Stormhawk]

@ Bruce its your choice in the in the end. All I wanted to say is that for the
project you do for yourself it really doesn't matter.

For the project you do for other people it also doesn't matter what style you pick because you have to code and do the style that the employer tells you. You have no choice when working for someone. Unless they tell you are free to use your own style.

@computerquip

I'm not talking about the use of functions or the actual coding per say. I am talking about the styling. Like you said about how you like to use spacing to make it clearer for yourself. or how one might to use a lot of pointers over someone else. Or how you might to use enumerator a lot more. Those kinds of habits is what i was really talking about. As time passes What you liked to do in the pass will change. Maybe you found a neat little trick with pointers that you love to use and ever since then pointers became you thing.

@ chrisname
You obviously didn't read the whole thing I wrote or you wouldn't have written most of what you had. So I won't even bother of replying back to it.




For the other people that are going on a rant. I said what I said and I will leave it as that. If your not able to understand the meaning of it then you hopefully one day you will. If not then oh well that is life. You can interpret and throw around the words however you like. I am not going to argue over it because you have a "point" to prove.

-sorry for any typos

[BruceJohnJennerLawso]

@DTSCode, sorry yes, its just some basic source files, not a library. There should be no issues that I'm aware of with linux, as its almost all written in the standard library. The only exceptions that you will probably want to comment out are the audio functions at the bottom of FRepo.cpp. Make sure to comment out:

void Play_sound_file(string FileLoc);
void Play_sound_file_def(string FileLoc);

I kinda felt all dirty linking to the windows stuff, but it was easiest way to make it happen quickly ;)

@Superdude, if you check the Testrun.cpp file it may help. In general though, the basic idea is that menus are objects of a menu class that jump back & forth by calling the pointer to another menu by comparing the id string of the menu. For example, in the Testrun cpp:

	CGenericMenu * main = new TCmdMenu ("CALCULATOR", "MAIN", "Main-Menu");
	CGenericMenu * graph = new TCmdMenu (  "MAIN", define_function_names, 10, false, "CALCULATOR", "Delta-Vee Calculation Menu");

for main (ie the main menu), "CALCULATOR" is the string id of the next menu down, "MAIN" is the string id of the menu itself, and "Main-Menu" is the title text that gets printed to screen in the command line between the double dashes.

under graph (meaningless name actually, the string id's are the only things that matter with menu navigation), define function names is an array of strings containing the keywords that can be used to access functions in that menu, (enter h or help in any menu to see a list of keywords available) 10 is the number of function keywords, "CALCULATOR" is the string id for the menu (again, important since it determines how other menus find a menu to call it)

When entering access keywords like "CALCULATOR" or "MAIN", always make sure to put it in uppercase. The program automatically switches input to uppercase in order to allow users to type in keywords in upper or lower case.

Hope that helps, best of luck :)

@Stormhawk, no worries, but this isnt the thread for it either. There should be a stickied thread on the topic of indenting somewhere around here.

[Dy7SLyTq]

@bruce: can you provide documentation?

for my two cents: i dont care how you wrote it (well the style at least). the only other major software i have seen use my style is sfml. people need to chill about the style. thats not as important as other things. there are tools for this. what is important is you said youll never make changes. not true. you might want to optimize this or rewrite that. so as long as it is readable to you is all that matters (with this project)

[Superdude]

I get 1 link error. Failure with linking with WinMM.lib? Do I need this? What code should I eliminate? I am using Windows 7 with Visual Studio 2012. :D

[S6k9GNh0]

Actually, I don't like spacing. I like code being compact. Specifically, as long as code is in a pattern, I'm generally okay with it but from personal experience, I find compact code quicker to read through. Of course, there are exceptions... I don't like long lines for instance.

myFunction(test1, test2, test3, test4, test5, test6, test7, test8, test9, test10, so_on);

Definitely do not like the above. I'll break it down into something that looks almost like a block of code.

myFunction(
    test1, test2, test3,
    test4, test5, test6,
    test7, test8, test9
);

My style is close to KNF except for what I do above with functions and templates (if I use templates... I tend to avoid them now adays).

[LB]

computerquip wrote:

(if I use templates... I tend to avoid them now adays).

Could you elaborate on this? I am interested in your reasoning.

[ne555]

You should care about file names being case sensitive
Don't ever use using in headers, it defeats the purpose of namespaces. Keep in mind that includes are just copy-paste, so the using is forced on the client.

I'm having issues compiling (gcc)

/*Frepo.cpp:481:56: error: no matching function for call to 
‘transform(
   std::basic_string<char>::iterator, 
   std::basic_string<char>::iterator, 
   std::basic_string<char>::iterator, <unresolved overloaded function type>)’*/
	{	transform (i.begin (), i.end (), i.begin (), toupper);	}

¿what is the testrun supposed to do?

-----------------------------Delta-Vee!_Integrated------------------------------

--Main-Menu--

that's all, and it seems that you handle <C-d> to do nothing.

	CGenericMenu * main = new TCmdMenu ("CALCULATOR", "MAIN", "Main-Menu");
	CGenericMenu * graph = new TCmdMenu (  "MAIN", define_function_names, 10, false, "CALCULATOR", "Delta-Vee Calculation Menu");
	(main)->menu();
}

it is obvious that it is leaking memory. You ought to `delete' what you `new', ¿but why are you using new?
`graph' is unused

The only time that you do delete is

//Graph_function()
		p_Vcheck = new int;
		*p_Vcheck = Compare_input_string_to_list(func_id, "E", "Q",  true);
		if (*p_Vcheck == 1)
		{	delete p_Vcheck;
			break;		}
		//here, memory leak 

sigh

[chrisname]

I would get into the habit of wrapping your heap-allocated memory in a std::unique_ptr or std::shared_ptr wherever possible. That way, it is guaranteed to be freed in any circumstance except where std::terminate or equivalent is called (in which case your program is insta-killed and the OS reclaims the resources anyway). You should probably default to std::unique_ptr for performance reasons (std::shared_ptr is reference counted which means it has to do extra checks) and only use std::shared_ptr if you know you might need to pass the pointer around.

[ne555]

So you put all the TCmdMenu in a global list.
- you never remove them from the list, so you may end with dangling pointers.
- I don't like the `search by name'. I've got a pointer from when the menu is created, ¿why can't use that?
- it seems that you need to modify both parent and child in order to insert a sub-menu.
- ¿why is it vector<CGenericMenu*> Linkslist; when the insertion code is in a subclass?
- `Execute_By_Id()' doesn't seem to use the object state
- ¿is it possible for two different sub-menus to have the same name?
- ¿is it possible to have several `terminal' menus? (the ones that execute functions)


Also, ¿why do you have global iterators?

By the way, you need a virtual destructor if you have a virtual member function.

[BruceJohnJennerLawso]

Oooh boy, lots today ;)

@Superdude, its the windows library needed for the wav player function. If you're having problems, just create a blank project in MVSC++ 2012, then import all of the files into it. WinMM.lib is located at...

C:\Program Files\Microsoft SDKs\Windows\v6.0A\Lib

at least for me. It might be at v7.0a for you if you're on MSVC++ 2012.

@computerquip, yes there will be documentation soon enough, but I'm too busy with school at the present moment. In another week or two I can probably get started on it. I apologize for the wait though...

@ne555, ...

I'm not sure if I follow what you mean by troubles with transform. Is it possible that you're using an older C++ standard for your standard library?

Uhhh, yes the testrun is somewhat pointless in having a main menu that goes virtually nowhere (no pun intended), but it acts as a demonstrator for more complex future programs. A better program might say, have multiple menus linked top down from the main menu, all executing different functions that work on different parts of the program.

Uhh yes, that is leaky isn't it. It doesnt appear to be causing problems ATM, since as chrisname noted, everything gets cleared when Exit(0); is called (theres a shutdown function in FRepo that does that). I have to double check my notes, but I believe polymorphic pointers have to be initialized dynamically in any case. At the same time, dynamic assignment might prove useful for runtime menu creation/deletion, but thats somewhat tricky (and can be done by just dumping the menu pointer in or out of the linkslist)

Must start patching things later today. Thanks for pointing that out.

On the topic of search by name, it might be possible to change that, but I generally founc comparing by name to be easier and safer than by direct pointer. It could...

Oh wait, I see now, pass neighbor pointers & their string ids at menu initialization. I guess, but how much do you think that will help performance wise? That would also break the simple workings of the help function as well.

I dont follow the last part about Execute_By_Id();. What do you mean?

I could add some sort of safety checker in the sub-menu checker to avoid the multiple sub-menu names thing. Not really that concerning of a hole, but constructor code can generally include lots of fail-safes without slowing things down much.

Yes it should be possible to have multiple function calling menus. All you really need to do is create another string array with the function ids, and add the right controls under Execute_by_id. I believe I have done at least one program that did that.

Global iterators, I know, will be corrected soon enough. I hadn't put much thought to that issue until recently. Same stuff with the virtual destructor, gotta check my notes again.

Thanks for all of the great feedback on the project. :)

[chrisname]

BruceJohnJennerLawso wrote:

Uhh yes, that is leaky isn't it. It doesnt appear to be causing problems ATM, since as chrisname noted, everything gets cleared when Exit(0); is called

I didn't say that to excuse you from deleting your heap-allocated memory, I said it so you wouldn't worry about what happens when the program gets killed abnormally and the destructors don't get called. You still need to get into the habit of doing something -- either using delete directly or taking advantage of library features like std::unique_ptr or language features like RAII in your own wrapper classes -- that prevents your program from leaking memory, otherwise you will always write crap software that leaks memory.

[BruceJohnJennerLawso]

I didn't say that to excuse you from deleting your heap-allocated memory, I said it so you wouldn't worry about what happens when the program gets killed abnormally and the destructors don't get called. You still need to get into the habit of doing something -- either using delete directly or taking advantage of library features like std::unique_ptr or language features like RAII in your own wrapper classes -- that prevents your program from leaking memory, otherwise you will always write crap software that leaks memory.

I'm not familiar with the wrapper classes idea that you mention, I'll need to experiment with that. In general though, what are the consequences of memory leaks?

[ne555]

> Oh wait, I see now, pass neighbor pointers & their string ids at menu initialization.
> I guess, but how much do you think that will help performance wise?
O(1) vs O(n). However your container is probably small, so it is not a big deal.
My complain is not about the performance, but how to `link' the menus

	CGenericMenu * main = new TCmdMenu ("CALCULATOR", "MAIN", "Main-Menu");
	CGenericMenu * graph = new TCmdMenu (  "MAIN", define_function_names, 10, false, "CALCULATOR", "Delta-Vee Calculation Menu");

iirc: `main' holds its name (MAIN) and the name of its child (CALCULATOR)
`graph' holds the name its name (CALCULATOR) and the name of its parent (MAIN),

A big problem is that the links are done just because of the name, so you must write them correctly. If you fail to do so, the error may be only caught at runtime.
Also, each time you add a child you must modify code in the parent.
Consider instead

CGenericMenu * main = new TCmdMenu ("MAIN", "Main-Menu");
CGenericMenu * graph = new TCmdMenu (main, define_function_names, 10, false, "CALCULATOR", "Delta-Vee Calculation Menu");

there we say that `graph' is a child of `main'. Because `main' must be declared we'll have a compilation error in case of a mistake.
Also, adding children is easier


> I dont follow the last part about Execute_By_Id();. What do you mean?
That if you do

main->Execute_By_Id("DV");
graph->Execute_By_Id("DV");
TCmdMenu().Execute_By_Id("DV");

they are all the same.

int TCmdMenu::Execute_By_Id(string function_link_name)
{	if ((function_link_name == Function1)||(function_link_name == Function2)||(function_link_name == Function3))
	{	DeltaVeeCalculation ();
		return 1;	}

iirc: you put here all the functions that may execute. That means if you want to add another menu with functions, you must modify that.
It's quite ugly.
Instead, you could store function pointers.

[cire]

So, I downloaded your code (why didn't you just include the code? I don't need your project files or binaries) and set it up to compile with my typical configuration. The result was:

1>c:\...\frepo.cpp(258): warning C4930: 'CFunction newFx0(void)': prototyped function not called (was a variable definition intended?)
1>c:\...\frepo.cpp(402): warning C4244: '=' : conversion from 'double' to 'int', possible loss of data
1>c:\...\frepo.cpp(411): warning C4715: 'Round_double_to_int' : not all control paths return a value
1>c:\...\frepo.cpp(428): warning C4715: 'Compare_string' : not all control paths return a value
1>c:\...\frepo.cpp(565): warning C4715: 'Check_vector_by_id_string' : not all control paths return a value
1>c:\...\frepo.cpp(577): warning C4715: 'Return_vector_element_pointer_by_id_string' : not all control paths return a value
1>c:\...\frepo.cpp(577): error C4700: uninitialized local variable 'output' used

All of which indicate serious potential problems with the code, and none of which I would allow in my code.

I attempted to comment out the implementation "Return_vector_element_pointer_by_id_string" because it was very clearly incorrect, but unfortunately it is apparently used somewhere as the linker complains about not being able to find it.

The function in question, reformatted:

CFunction* Return_vector_element_pointer_by_id_string(string i, vector<CFunction> v)
{
    // v is a copy of a vector.  The address of any CFunction obtained in this function
    // will cease to point to a valid CFunction object when the function returns, as the CFunction
    // pointed to will be destroyed.

    vector<CFunction>::iterator iter;
    CFunction * output;
    bool is_go;
    for (iter = (v.begin()); iter != (v.end()); ++iter)
    {
        is_go = Compare_string(i, (iter->id));
        if (is_go == true)
        {
            output = &iter.operator*();
            return output;
        }
        else if ((iter == v.end()) && (is_go != true))
        {
            // It is not possible for this code to be reached.
            return output;
        }
    }
    // What is returned if we reach this line?
}

Please see the comments therein.

[chrisname]

BruceJohnJennerLawso wrote:

I'm not familiar with the wrapper classes idea that you mention, I'll need to experiment with that

Well, you can use std::unique_ptr or std::shared_ptr for most dynamic pointers, but sometimes you might want to write your own classes that automatically free the allocated memory in the destructor (which is what the std::*_ptr classes do). Take this code, for example:

p_Vcheck = new int;
*p_Vcheck = Compare_input_string_to_list(func_id, "E", "Q",  true);
if (*p_Vcheck == 1)
{	delete p_Vcheck;
	break;		}

You could change it to

p_Vcheck.reset(new int);
*p_Vcheck = Compare_input_string_to_list(func_id, "E", "Q",  true);
if (*p_Vcheck == 1)
{	break;		}

and it wouldn't leak memory because the destructor of std::unique_ptr would be called as soon as it went out of scope and call delete on the pointer. Also, you'd have to change the type of p_Vcheck to std::unique_ptr<int> and include the <memory> header, but otherwise, your code would pretty much stay the same.

That said, I have no idea why you were allocating that pointer to int on the heap in the first place. You should use stack variables unless you're sure you need dynamic memory. Not only does it look weird and take longer (since heap allocation is slow), it also uses twice as much memory, because not only are you allocating 4 bytes (probably) on the heap for the int, you're also allocating 4 bytes on the stack for the pointer.

what are the consequences of memory leaks?

It depends on the runtime of your program and the severity of the leak (which tend to correlate). Very small leaks or leaks in programs that don't last for long don't have any real consequences except that it makes you look bad. Large leaks or leaks in programs that are expected to run for a long time have more severe consequences. It could crash your program, and it could get to the point where other processes can't allocate any more memory when they need to, meaning they become unstable or crash. If it's really bad, it could prevent the OS allocating any more memory at all, so the user wouldn't be able to run any more programs and wouldn't be able to stop yours from running. In theory, you could even destabilise or crash the OS. Most memory leaks aren't that bad, though; Firefox is somewhat notorious for minor memory leaks which decrease the performance of the system (and of the program) by using up lots of memory. Again, in most cases, it just makes you look bad and makes people complain about your software being poor quality.

[S6k9GNh0]

L B, comes with not using C++ in general. I've not been doing a lot of coding but when I do, it's usually in strict C99 to mess with in multiple languages or ASM.

[helios]

If it's really bad, it could prevent the OS allocating any more memory at all, so the user wouldn't be able to run any more programs and wouldn't be able to stop yours from running. In theory, you could even destabilise or crash the OS.

Nah. The specifics of the behavior of a leaky program depend on the sizes of the leak and of the address space. Generally speaking, though, a leaky program shouldn't kill the system. Leaked memory is inaccessible, so the OS is likely to swap it out once it can no longer allocate more physical memory. System-wide performance may suffer due to thrashing, but that's about it.