Re: how to remove code duplication
- From: Nick Keighley <nick_keighley_nospam@xxxxxxxxxxx>
- Date: Mon, 11 Aug 2008 02:00:08 -0700 (PDT)
On 11 Aug, 07:46, arnuld <sunr...@xxxxxxxxxxxxxxx> wrote:
I have created a program which creates and renames files. I have
described everything in comments. All I have is the
cod-duplication. function like fopen, sprint and fwrite are being called
again and again.
I know to remove code-duplication I have to make functions and pass
arguments to them but I am not able to think of a way doing it. Can you
post some example for me, out of this code:
<snip waffly comments>
<snip code>
you have a 600-line main function!!!!
As a rule of thumb start to ask yourself if the function could be
split
at say 10-20 lines. A 50 line function isn't always wrong (eg. big
switch
statement) but an alarm bell should be ringing.
Step 1 write a test program that verifies your program is correct.
Perhaps base it on this skeleton
/* runs a test on the logger
the log directory contains the in files before the test
the data from the data directory is read
the log directory should containing the same files as the out
directory
*/
void run_log_test (const char *test_name)
{
char test_dir_in [80];
char test_dir_out [80];
sprintf (test_dir_in, "test%s/in", test_name);
sprintf (test_dir_dat, "test%s/data", test_name);
sprintf (test_dir_out, "test%s/out", test_name);
clear_log_directory();
copy (test_dir_in, log);
read_data (test_dir_dat);
match_dir (log, test_dir_out); /* calls assert if any files
don't match */
}
void test_logger()
{
run_log_test ("1");
run_log_test ("2");
run_log_test ("3");
}
it should be designed so you don't have to check data
by hand- the program does it all for you.
Write some tests that verify it behaves correctly for various
scenarios.
eg. no 0.log short input (no file roll-over)
0.log present short input
medium input (rolls over to 1.log)
long input (less than log max files created)
very long input (files have to be deleted)
anything else that might stress your program
step 2 run the tests
step 3 identify short bits of repetitive code.
create functions from the repeated code.
call the functions where the code was repeated.
step 4 run the test
At the moment your code is so tangled its hard to pick out
simple bits of repetitive code.
I'd "wrapper" calls like these
if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SPRINTF ERROR -- near line 217");
exit( EXIT_FAILURE );
}
void make_name (char *log_name, int log_cnt)
{
if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SPRINTF ERROR -- near line 217");
exit( EXIT_FAILURE );
}
}
which doesn't save you much but you call them a lot.
Similarly fopen(), fwrite(), fclose().
As a rule of thumb if you find yourself using block comments
like
//
=========================================================================
===========
//========================== RENAME MECHANISM ========================================
then that is *begging* to be turned into a function. Even
if it only appears once it will simplify you main.
This seems to appear a few times
// open file and append the data
if( ! (fp = fopen(log_name, "a")) ) // S2 -- case a
{
perror("FOPEN() ERROR whne log_cnt == 0");
exit( EXIT_FAILURE );
}
if( fwrite(log_recv_arr, 1, log_recv_size, fp) != log_recv_size )
{
perror("FWRIRE ERROR in log_cnt == 0");
exit( EXIT_FAILURE );
}
if( fclose(fp) )
{
perror("FLOSE() ERROR near line 187");
exit( EXIT_FAILURE );
}
// update log_size
if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
printf("%s written = %ld\n\n", log_name, log_size);
// update the log_cnt
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++log_cnt;
}
Try to identify higher level funtionality. The following code is
uncompiled.
int main (void)
{
FILE *stream;
get_log_stream(stream); /* TBD */
process_log_data(stream);
return 0;
}
void process_log_data(FILE *stream)
{
int log_cnt = 0;
int i;
FILE *curr_log;
while (more_stuff)
{
if (!log_file_exists(0))
create_log(0);
curr_log = open_log(0);
if (store_records (curr_log, stream) == STREAM_EMPTY)
return;
/* current log full- move to next log */
fclose(curr_log);
/* rename logs */
for (i = log_cnt; i >= 0 i--)
rename_log (i, i + 1);
log_cnt++;
/* TBD handle too many log files */
}
}
/* store recored in the current log until either
- stream exhauseted
- log full
*/
Store_result store_records (curr_log, stream)
{
/* TBD */
}
/* primitives */
int log_file_exists (int n)
{
FILE *f;
make_name(n)
if ((f = fopen(make_name(n), "w") == NULL)
return FALSE;
fclose(f)
return TRUE;
}
char *make_name(int n)
{
static name[80];
sprintf (name, LOG_NAME, n);
return name;
}
still rather messy but I probably did it the opposite
way to you. You wrote pages of code and then thought
"how do I modularise this?". I thought "what is the program
trying to do it and what functions whould make it
easy for me to write it".
I quickly realise I want to open files, rename files etc.
So at the moment process_log_data represents the guts of the program.
The log renmae loop should probably go into a function
and the handling of too many logs.
--
Nick Keighley
.
- Follow-Ups:
- Re: how to remove code duplication
- From: arnuld
- Re: how to remove code duplication
- From: Andrew Poelstra
- Re: how to remove code duplication
- References:
- how to remove code duplication
- From: arnuld
- how to remove code duplication
- Prev by Date: Re: testing fclose...really necessary
- Next by Date: Re: how to remove code duplication
- Previous by thread: how to remove code duplication
- Next by thread: Re: how to remove code duplication
- Index(es):
Relevant Pages
|
