[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Ok, new problem

From: Przemek Klosowski
Subject: Re: Ok, new problem
Date: Mon, 19 Nov 2007 14:07:25 -0500 (EST)

I probably sound like an old grumpy man saying this, but you should
rethink the style of your programming. You tend to use a single large
subroutine to do the entire task, which makes it hard to find and
debug problems---you can't see what's happening inside your code.
This is actually not even specific to Octave---having worked with
scientific programming for a long time, I see a lot of Fortran, C,
Octave and whatever else, written in this 'On The Road' streaming

You need to break it up into small, agile, testable little
pieces. This is good for several reasons, some actually relevant to
the questions you posted:

- it's easier to see logical and syntax error in smaller modules 
- it's easier to test small modules that return intermediate values
  that you can examine for correctness
- you exploit patterns and avoid cut-and-paste repetition errors
- you can better see relationships and commonalities between modules.
- it's easier to evolve code

For instance, note that you probably made a mistake for the third
term: you have f3=f*6, where I think you meant f*3, but it's hard to
see when you write it like you did.. Also, my version of octave
requires the following usage of wavwrite():

       wavwrite (filename, y, samples_per_sec, bits_per_sample)

the ordering of parameters you used in your program doesn't work for me.

People did studies on programming ergonomics, and came up with a
rule of thumb that code modules should have 12 lines or less. Of
course some of those lines may be macros, or subroutine calls, so 
this doesn't limit the complexity of your code---but you have to
give some thought to how you split it up.

I would rewrite your code thus:

    function w=oneChirp(t,tau,A,f)

    function w=sinte(f,Fs,T)
       t  = 0:1/Fs:T;
       w  = oneChirp(t,.044,28184,     f) + \
            oneChirp(t,.036, 7943.3, 2*f) + \
            oneChirp(t,.048, 2818.4, 6*f) + \
            oneChirp(t,.015, 7943.3, 4*f) + \
            oneChirp(t,.021, 1778.3, 5*f) + \
            oneChirp(t,.018, 2818.4, 6*f) + \
            oneChirp(t,.016, 1258.9, 7*f) + \
            oneChirp(t,.017,  794.3, 8*f) + \
            oneChirp(t,.038,  100.0, 9*f) + \
            oneChirp(t,.017,  562.3,10*f) ; 

    Fs = 44100;
    f=300 ; 

Note that you can plot and see the resulting waveform and even plot
component parts:

    plot(t, w)

so you exploit the scriptable, immediate execution nature
of Octave for ad-hoc debugging and manipulation.

Your parameters A and tau---if you didn't just pull them out of your
hat, you probably would calculate them, and they would come out of
an array. It'd be easy to rewrite the code above to use it:

   for i=1:10
     w += oneChirp(t, tau(i), A(i), f*i)

or even vectorize by changing oneChirp() to take array values as
parameters, and return the sum of terms directly.

There are some disadvantages to very modular style. You tend to use
more function parameters (note my use of Fs and T in the sinte()
call)---the only alternative being global variables, which would
not be recommended. Also, too many modules and function calls can be slow,
but that is not a concern here.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]