I don't see any point in breaking a function into smaller pieces just because it's too long. It's one logical concept. You end up massaging and making things more complex for no real benefit.
I appreciate everyone's comments.
Printable View
I don't see any point in breaking a function into smaller pieces just because it's too long. It's one logical concept. You end up massaging and making things more complex for no real benefit.
I appreciate everyone's comments.
The benefit is that you end up with smaller pieces that are easier to verify correct by inspection, and which can be more easily unit tested. Besides this, the abstraction means that a reader can more easily "ignore" parts of the functions that calls these smaller pieces because he/she already knows what they do, and either knows that they are correct or will verify their correctness if need be.Quote:
Originally Posted by jlewand61
I'm curious, did you really come here for advice on your first program? You seem to be finding reasons to disagree with everything we're saying.Quote:
Originally Posted by jlewand61
Beside bad coding style observations (some of them already pointed above) like bad naming, global variables, ignored warnings, too complex functions, "java-style" brackets, reinvented wheels and so on, I have an additional one.
Let's make a simple program based on few lines from your code (being closer to "my very first C program" ;)).
My_Test_Console.cpp compiles with no problem.Code:// My_Test_Console.cpp
#include <windows.h>
#include <tchar.h>
#define MAXDLA 40 // max # entries in drive-letter array
#define MAXSOPTA 40 // max # entries in space option file array
TCHAR TCHbuff[MAX_PATH]; // GetLogicalDriveStrings
TCHAR dlal[MAXSOPTA][3]; // driveletter 00 - NO COLON
TCHAR dlay[MAXDLA][12]; // drive type - FIXED -room for REMOVABLE?
int dlat = 0; // num of entries - or next element to populate
int _tmain(int argc, _TCHAR* argv[])
{
// populate driveletter and drive type
dlal[dlat][0] = TCHbuff[0];
dlal[dlat][1] = _T('\0');
_tcscpy_s ( dlay[dlat], _T("FIXED") );
return 0;
}
However, if change the source file extension from ".cpp" to ".c" telling Visual Studio to use C compiler it gives an error.
Why the C compiler complains while the C++ one has nothing to say?
Let's take a look in _tcscpy_s (strcpy_s, wcscpy_s) documentation: http://msdn.microsoft.com/en-us/libr...(v=VS.80).aspx
We can notice overloaded functions strcpy_s/wcscpy_s.
In the above program, we've passed two parameters, then C++ compiler choses the (template) one which requires two parameters.
The C compiler gives an error because it expects three parameters.
To keep in mind
Unlike C++, C language has not:
- function overloading;
- template functions;
Conclusion
- if write a C-style program, does not mean it will be always obviously C compiled;
- although related, C an C++ are different programming languages;
- everytime we are using a library function, we have to pay a serious attention to its documentation.
Speaking of this, jlewand61, in a message board community that you participated in, I responded to one of your threads with this:Quote:
Originally Posted by GCDEF
Quote:
Originally Posted by jlewand
Quote:
Originally Posted by jlewand
Maybe you would like to keep an open mind about the suggestions made here? If you are indeed experiencing a "culture shock", there may be an automatic reaction to fall back on what you know from your assembly language experience, even if it does not necessarily apply when programming in C (or C++).Quote:
Originally Posted by laserlight
Yeah, unless you're talking about COBOL or something, recursion and local variables shouldn't be an alien concept. They're pretty common in just about all modern high-level languages that I know of. Recursion is a basic theme in any algorithms course.Quote:
Originally Posted by laserlight
Recursion is an alien concept from a programming standpoint. In all the assemblers I've worked on, there is no such thing as "local" or "global" variables. Variables are simply addressible storage.
Not from a programming standpoint it isn't. It's a fundamental concept taught early in any programming course. Assembly programming is a long way removed from any higher level language. If that's all you have experience with, you'll need to wipe your brain clean when it comes to approaching problems and start again.Quote:
Originally Posted by jlewand61
We are talking here about C and C++ languages and not about Assembly.Quote:
Originally Posted by jlewand61
Experience, not "reasons". It shouldn't bother anyone whether I take advice or not. It wouldn't bother me if someone deflected my advice. I've sincerely asked to help me get into good C coding habits. C is a new animal and I'm trying to develop good habits from the start.Quote:
Originally Posted by GCDEF
I was resopnding to the comment that recursion or local/global variables should not be foreign concepts. They ARE. Quite so.Quote:
Originally Posted by ovidiucucu
Well then, just forget the old habits.Quote:
Originally Posted by jlewand61
Like somebody already said, C is not Assembly and vice versa.
In practically every single mid to high level language, there are such things as local and global variables. Whether it is C, C++, Pascal, FORTRAN, VB, C#, Java, I could go on and on listing all the languages where local and global variables exist.Quote:
Originally Posted by jlewand61
In none of these languages is it desirable to "globalize" your variables to such an extent as your code does. It isn't just the few voices here saying it, every expert will tell you the same thing.
Regards,
Paul McKenzie
Just to note: At least MASM does support local variables since a bunch of versions. I've never used that feature myself, though, since I never felt the urge to write anything in assembler that needs local variables or does recursion.Quote:
Originally Posted by jlewand61
And local variables are even so non-strange that there's even hardware support for them to some extent in some CPU architectures (including the x86, BTW ;)).
If a function is long, yet it serves a single purpose and does not include code duplication, then I tend to agree. Separating code just to make each function fit on a single screen doesn't help readability IMO.Quote:
Originally Posted by jlewand61
However, if there is code duplication like in various places in your code, then that should be factored out even in small functions. This makes the program easier to maintain, test and understand.
Something that hasn't been mentioned (unless I missed it) is that several of your functions have multiple exit points (i.e. return statements). Also, you are calling exit in quite a lot of places. In a C++ program that would be OK, since resources should be managed using RAII classes anyway. But in C that's not possible, so having multiple exit points makes it very hard to properly manage resources in all possible execution paths.