Too Many Variables?

[Somersett]

Well I'm about 1000 lines of code into my first program. After having recently "compartmentalized it" into small functions for readability's sake, I now find myself passing upwards of 8 or 9 variables into them. As I'm thinking of how the program will develop, I realize I'm probably going to be incorporating even more variables that will need to be passed.

It struck me that this is something I should utilizes structures for, so I could just be passing one or two arguments that might contain these 10 or more variables. Am I write in thinking this? I've never utilized structures and obviously I'm very new. I don't want to go renaming everything into structure form if this is a weird use of them.

[Narue]

>Am I write[sic] in thinking this?
Makes sense to me. Though structures are best used for closely related data rather than just a variable dump. If the relationships aren't there, you may need to consider how you're structuring your program and redesign if necessary to streamline the flow.

[ArkM]

Don't flatter yourself with the program of 1000 lines: it is a small program (max 2 days on implementation ;)). So the true problem is not a number of variables per se.

It's not a common rule, but a well-designed function has 1-5 parameters. If it has 10 or more parameters then functional decomposition was made badly. There is the better way to do it. Another source of troubles in that case is a bad data (or object) model design.

For example, in a recent thread on this forum the author passed 10 arguments presented CPU emulator registers. In actual fact all these variables are corresponded to the only artifact - CPU register file. It's the only array variable. Moreover, we get much more clear and compact code with register file abstraction.

Never group variables into structures on formal basis (to reduce parameter/arguments lists). Every structure is a data abstraction. If you don't know what's this abstraction, don't group such variables.

So too many variables feeling is an alarm bell: it's a good time to come back for redesign and/or refactoring.

Don't worry: it's absolutely normal situation...

[Narue]

>It's not a common rule, but a well-designed function has 1-5 parameters.
I'd say it's fairly common to restrict the number of parameters with a guideline. I usually recommend that you seriously reconsider what you're doing when you exceed three, and more than seven local variables suggests a need for refactoring.

[ArkM]

The number of parameters best practice depends on the problem domain. For example, there are more parameters in routines of scientific graphics or linear algebra packages (not only from early Fortran heritage ;)).

[Somersett]

Never group variables into structures on formal basis (to reduce parameter/arguments lists). Every structure is a data abstraction. If you don't know what's this abstraction, don't group such variables

Thanks, this is basically what I was asking.streamline the flow.refactoring.

The only thing I can think of in terms of redesign is to break the functions down into itty-bitty pieces, which I'm reluctant to do because I feel like there's too many; I've already made 15 functions in this 1000 lines of code (trying to impress you again). I don't know what refactoring is and 'streamline the flow' sounds like a christian slater movie.

I guess my questions are centering around basic design, because I'm not exposed to the wisdom of other programmers outside of forums like these.

[Narue]

>The only thing I can think of in terms of redesign is
>to break the functions down into itty-bitty pieces
That's often no better than one monolithic function. To properly streamline your code, you may need to redesign the entire structure such that each function has a well-defined purpose without tasking your brain in terms of excess variables or tons of unnecessarily small functions. Without seeing your code, I can't really offer better advice.

>I've already made 15 functions in this 1000 lines of code (trying to impress you again).
In terms of size, I work with code bases that have millions of lines on a daily basis. 1000 lines won't impress me unless it's wicked cool code from beginning to end. ;)

[Somersett]

Without seeing your code, I can't really offer better advice.

void InsertEvent (char * channel, char * reserve, int selected, int * eventcount, int * wavelength, int * seconds, int * samplecount, HWND EDC_Wav, HWND EDC_Sec)
{
	// will shift eventcount, wavelength, seconds, samplecount

	int i ;
	int changepoint, reservepos, new_endpos, shiftlength ;

	//mark point of change in samples (INCLUDE old wave)

	if (selected != 0) changepoint = samplecount [selected - 1] ;
	else changepoint = 0 ;

	//copy data & clear channel

	reservepos = 0 ;

	for (i = changepoint ; i < samplecount [*LASTEVENT] ; i++)
	{
		reserve [reservepos++] = channel [i] ;
		channel [i] = 0 ;
	}

	//shift everything up

	for (i = *eventcount ; i > selected ; i--)
	{
			wavelength [i] = wavelength [i-1] ;
			seconds [i] = seconds [i-1] ;
			samplecount [i] = samplecount [i-1] ;
	}

	(*eventcount)++ ;

	//get new wavelength & seconds, replace in old eventcount

	ReadEdits (EDC_Wav, EDC_Sec, wavelength, seconds, selected) ; 

	//produce new wave at cleared location
	
	new_endpos = // get end position of new wave
		SineWave (channel, wavelength [selected], seconds [selected], changepoint) ;


	samplecount [selected] = new_endpos ;

	//insert rest of channel after, clear reserve

	for (i = 0 ; i < reservepos ; i++)
	{
		channel [new_endpos++] = reserve [i] ;
		reserve [i] = 0 ;
	}

	//shift samplemarkers

	shiftlength = new_endpos - samplecount [selected + 1] ;

	for (i = selected + 1 ; i < *eventcount ; i++)
		samplecount [i] += shiftlength ;

	return ;
}

Here's the one that takes the most arguments, esp because it reads from the GUI (Win32).

My program currently manipulates one channel of audio samples in the form of simple sine waves. The wavelength, seconds, & samplecount fields are all attributes that correspond to a particular 'eventcount'. This particular functions passes 'selected', meaning from the graphic representation of the channel, then shifts everything up and creates a new event from the user's input in the HWNDs.

I created a static 'reserve' channel so that I wouldn't have to allocate memory every time that data is copied. Other than that, the variables don't seem extraneous and they're all used fairly compactly with each other in this procedure, so that breaking it up would seem blatantly arbitrary.

[Narue]

My 5 second solution[1] is to encapsulate the event stuff in one structure and higher up, a list of events and the channel info in another structure. For example:

struct event {
  int *wavelength;
  int *seconds;
  int *samplecount;
};

struct channel {
  char *channel;
  int selected;
  int eventcount;
  event *events;
};

The reserve parameter strikes me as sufficiently global to store as a global variable (make it static if you can so it's restricted to a single file). Doing all of this drops your parameter list to three (assuming my 5 second assumptions about your data are correct):

void InsertEvent(struct channel *channel, HWND EDC_Wav, HWND EDC_Sec);

You can further shrink it by encapsulating all of the Windows-specific data that's related in a structure as well:

void InsertEvent(struct channel *channel, struct EDC_Data *data);

[1] I spent five seconds thinking about it.

[Somersett]

Okay, so it does seem to be a matter of creating structures for the purpose of lowering the parameter list (and general readability). This is what I imagined when I first posted. Thanks.

[minas1]

You could use arrays as well.

[BestJewSinceJC]

Why would you add an unnecessary level of abstraction (creating the structures) if you don't need to? The point of a structure is to group related data, not to shove **** together in order to reduce # of parameters

[Somersett]

an unnecessary level of abstraction (creating the structures)

When is creating structures necessary? What would you propose to my original post?

[ArkM]

I agree with BestJewSinceJC (also see my previous post ;) ).

IMHO it seems that you don't understand the meaning of Narue's improvisation (may be I'm wrong). It was an attempt to invent proper abstractions (event, channel) for totally unstructured low-level code with chaos of unrelated variables. Parameter lists reducing is an inevitable result but not the main goal of this approach.

[Somersett]

It was an attempt to invent proper abstractions (event, channel) for totally unstructured low-level code

You mean create structures.with chaos of unrelated variables.

Is the code I posted "chaotic"? How so?

What makes certain variables "related" or "unrelated"? I.e., what deems them worthy or unworthy of creating structures? My idea was that I might group together those variables which seemed to get passed to functions with each other most often.

I know this is going over my head because I feel like I'm still asking the same question as when I started. I appreciate your replies though.