Index

Subject : Re: LUG: Coding Horrors -- post your evils

From : Richard Carter <rwcarter@ncsu.[redacted]>

Date : Thu, 06 Aug 2009 08:11:31 -0400

Parent


It looks to me like you could refactor a couple of nice ways. The
names can vary based on what you want to call them but here's the
basics:

template struct Coords<typename T> {
T x, y, z;
}

template struct ThreeByThreeMatrix<typename T> {
Coords<T> one, two, three;
}

void Aniso_Pa_Field_Update(int z_min, int z_max, int x_min, int x_max,
Coords<COMPLEX**> p,
Coords<COMPLEX**> past_p_a,
Coords<COMPLEX**>past_q,
ThreeByThreeMatrix<COMPLEX**> r,
ThreeByThreeMatrix<COMPLEX**> cep,
ThreeByThreeMatrix<double**> delta,
ThreeByThreeMatrix<COMPLEX**> cxi,
double th, double S)

Then of course, add easy constructors on the two structs so that you
can pass in data easily without the calling code needing to declare 7
explicit variables; unless it's messy, in which case the variables may
be worth the maintainability.

And if you wanted to go one step further you could make a MinMax
struct that holds an int min, max; to condense the first 4 parameters
into 2.


Ricket


On Thu, Aug 6, 2009 at 6:49 AM, Daniel
Underwood<daniel.underwood@ncsu.[redacted]> wrote:
>> I'm up for any suggestions on how to clean this up,
>
> Some scientific computation routines are almost unavoidably
> complex/messy. What do you hope to achieve by cleaning up the code? �Is
> it primarily for your own benefit in avoiding errors? �Is it primarily
> in order for others to be able to modify your code? What?
>
> If you modularize and isolate the "messiness", and if the code works and
> is efficient, there should be no problem. The code that calls these
> functions, however, should be cleaned up, especially if there are calls
> in multiple locations.
>
> You might clean up the Aniso_Pa_Field_Update() parameters by packaging
> the arguments into a struct and then changing this:
>
> void Aniso_Pa_Field_Update(int z_min, int z_max, int x_min, int x_max,
> COMPLEX** pxa, COMPLEX** pya, COMPLEX** pza,
> COMPLEX** past_pxa, COMPLEX** past_pya, COMPLEX** past_pza,
> COMPLEX** past_qx, COMPLEX** past_qy, COMPLEX** past_qz,
> COMPLEX** rxx, COMPLEX** rxy, COMPLEX** rxz,
> COMPLEX** ryx, COMPLEX** ryy, COMPLEX** ryz,
> COMPLEX** rzx, COMPLEX** rzy, COMPLEX** rzz,
> COMPLEX** cepx1, COMPLEX** cepx2, COMPLEX** cepx3,
> COMPLEX** cepy1, COMPLEX** cepy2, COMPLEX** cepy3,
> COMPLEX** cepz1, COMPLEX** cepz2, COMPLEX** cepz3,
> double** deltaxx, double** deltaxy, double** deltaxz,
> double** deltayx, double** deltayy, double** deltayz,
> double** deltazx, double** deltazy, double** deltazz,
> COMPLEX** cxi11, COMPLEX** cxi12, COMPLEX** cxi13,
> COMPLEX** cxi21, COMPLEX** cxi22, COMPLEX** cxi23,
> COMPLEX** cxi31, COMPLEX** cxi32, COMPLEX** cxi33,
> double th, double S)
>
> to this:
>
> void Aniso_Pa_Field_Update(structT PTRS, int z_min, int z_max, int
> x_min, int x_max, double th, double S)
>
> Granted, this would make the body of Aniso_Pa_Field_Update() a little
> more "messy", but the increased messiness there would be justified by
> the decreased messiness in the calling code.
>
> I'm far from an expert, and it's 6:45am and I haven't had my morning
> coffee, but these are my initial thoughts.
> --
> Daniel Underwood
> North Carolina State University
> Graduate Student - Operations Research
> email: daniel.underwood@ncsu.[redacted]
> phone: XXX.302.3291
> web: http://www4.ncsu.edu/~djunderw/
>
>



Replies :