First of all there is no coding problem neither any other problem with this code
This is my first user-oriented program .. well i mean my first project with C..

I'm into C for about 2 months now ..

this is a very simple (p.s. maybe a huge n childish coding) ...
n i would like ur valuable comments .. say any improvements n better way of coding a particular part
would be of great help ...

Thanks in advance ....

ATTACHED : database.txt


I wanna use my own made header files but is clueless about how to make-em...

Thank u once again

Attached Files

DATABASE.txt (29.1 KB, 20 views)

I'm not sure that I'd use the phrase "no coding problem" in this particular case. However, I may have some useful comments.


Some of these errors are obviously related to your use of a compiler that has a non-standard "conio.h" file for DOS applications. One recommendation would be to avoid using non-standard code of any type.

There are numerous variable naming "conflicts." A variable naming conflict exists when you call something one thing, then use it under a different name in a way that adds confusion rather than reduces it. For example, you declare a global (probably a bad idea to use a global) "info" structure and an array of 100 are called a record.

I don't know why record[INF] (where INF is 100) is any more or less useful than address[100]. There is an old saying in C programming that goes something like this: The only magic numbers that should be used in a program are 1 and 0.

How meaningful are variable names like ph_r, ph_o, ph_m? Is there some reason that you can't spell them out...or at least comment their meaning in the declaration of the structure? I think that with enough thinking that we can figure them out, but is that what you want your LGPL'd code to be? Something that others have to work harder at to figure out the meaning of it?

What is a "filen" ...is it a file number? What is the difference between its members file_name and fname?

Since this is no longer the "old days," please remember that main always returns int.

I don't particularly care for the declaration of functions inside of main as found in:


I really don't care for the notion of declaring the return type for xstrncpy as char and a char variable named "e" ...what possible meaning does "e" have? You really should separate each variable declaration to its own line. It makes your code cleaner and easier to read and understand.

The idea of hardcoding "c:\\windows\\" into anything in a C program sounds rather in poor taste for portability, which is one of the primary reasons we use C to write code.

I'd seriously consider avoiding things like:

printf("\t NAME, PHONE NO OR ROLL NO. U CAN ALSO MANAGE YOUR\n");

...where you use "U" in your user output. We at least try not to act like we're totally dysfunctional, whether in code, email or on forums such as this one.

I'd also look for obvious typo/spelling errors such as:

printf("\t DB MANAGENT :\n");

Code like this:


Can be in "violation" of an old saying that goes something like: There should only be one return statement in a function. Also, the idea of checking the range of "ch" in this manner is fairly ugly. You have a magic number "5" that has meaning only because you have 4 menu choices. If, in your later work, you decide to add or remove a menu choice, now your magic number needs to be changed. Perhaps an alternative is to assign ch to zero and return it at the end of the code whereby the middle of the code may modify its value to something meaningful to the function?

There is another problem with the "design" of the code from the perspective that says that someone using this function must know that "4" means to do something. This places a dependency on the code that uses it and should the "menu" ever change, so must the user. Also, the user must know what the menu "internals" do and rely on them to not change or do something in a different way. In a way, it is like building a link chain of dependencies. If one link in the middle requires work, all of the other links "fail" to support the goal of the chain. I'd avoid these. A practical solution might be to implement an array of function pointers whereby the mapping of function pointer (work) to menu choice (user input) is maintained in a suitable structure that is initialized on program startup?


This code does not check to see if buffer "x" is large enough to hold the contents of the string literal and buffer "y." I think that I'd prefer to see some kind of bounds checking here.

There is another problem a bit later in the "n" cousin version of this function:


File names can start with numbers and certain non-alphabetic characters. For example: 1.txt is a valid file name. " ".txt is a valid file name (space in quotation marks). Of course, your program doesn't have to accept them, but I might be insulted if I was a user and you contradicted something that I already know about my system. Besides, what do you care about what the user decides to name a file? Programs rarely care about file names as long as they can obtain a valid file descriptor, they're happy. File names are for users.


Here you call the function datab and then proceed to tell us that it is a management function. Why not call it "manage_db" or something that is more meaningful to better indicate its purpose?

I don't want to sound completely negative in my critique of your code. The fact that you wrote some 1110 lines in it is an indication that you're interested in what this code does. I encourage you to continue to seek ways to improve your code and your skillset. Your post indicates to me that you are working toward that goal. Keep up the good work!


:davis:

First of all i want to thank you for your immense patience for reading the huge code and providing such a useful analysis and true criticism.

Well frankly speaking my compiler did not gave any sort of warning nor error while compiling the stuff .. i use Borland Turbo C++ V3.0

I'll take care of all your advice while coding any programs in the future


I'm sorry but i couldn't grab what do you mean by non-standard "conio.h" file.. i'm at total loss if you could please explain it..

There will be no more spelling mistakes and variables will clearly describe what it suppose to do..

Okie-dokie .. roger that.. SIR

Once again i would like to thank you .. and please criticise for what is wrong it may be harsh but i need to correct them before the errors crept in my coding ...

GOLDFISH

TC++ v3.0 is a very "old" compiler and isn't up-to-date with the C Standard. The file conio.h is NOT part of "Standard C" (or Standard C++, for that matter). If you use conio.h in your programs, you are using non-Standard code. In some cases, that may be perfectly acceptable, however, it will also require others to have a compiler very similar if not the exact same version as yours to work with your code.

If you plan to LGPL your code, you will probably want for it to be as useful as possible to as large an audience as possible. With that consideration in mind, I'd recommend using only Standard C and/or Standard C++. In other words, the code should compiler using any reasonably standards-conformant compiler.

I believe that it has been well over 10 years since TC++ v3 was released. The most recently ratified C standard was 1999. This means that if you are using a compiler released before about 2000, that it is very likely not fully conformant with the standard.

You may want to visit www.bloodshed.net for an updated compiler that will be standards conformant...and free.


:davis:

Thanks for ur help .. i'll upgrade my compiler ... thanx DAVIS

One more thing i'm a bit confused .. you said main always return an int type value ..

Q1.> To whom it returns the value ?
Q2.> Can't main() function be a void one?
(i mean like )

Goldfish

A1: It returns the value to its caller, just like any other function. How do you start your program? Let's say that you're using MSDOS or a Unix shell. So you have an opened "console" and you type in:

C:\myprogram.exe (DOS) or
./myprogram (UNIX)

The return will be made to the caller. In this case, the "shell" application.


A2: The C99 standard requires main to return an int. However, most of the compilers that I've seen only produce a warning if main is declared with a void return type. In other words, declare main to return int so that your code is up-to-date with the standard.

Also, I'd dispense with the "ur" and "thankx" "internet-speak."


:davis:

Further explained here

__________________

Cow: You're a lawyer too?
Mooseblood (mosquito): Ma'am, I was already a bloodsucking parasite. All I needed was a briefcase!