octave-bug-tracker
[Top][All Lists]
Advanced

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

[Octave-bug-tracker] [bug #48774] Functions for moving statistics


From: Juan Pablo Carbajal
Subject: [Octave-bug-tracker] [bug #48774] Functions for moving statistics
Date: Wed, 19 Dec 2018 08:55:33 -0500 (EST)
User-agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:64.0) Gecko/20100101 Firefox/64.0

Follow-up Comment #25, bug #48774 (project octave):

As a concrete example of the problem mentioned in the previous comment, see
this snippet:

  ## Obtain function for boundary conditions
  if (isnumeric (bc))
    bcfunc = @replaceval_bc;
    bcfunc (true, bc);  # initialize replaceval function with value
  else
    switch (tolower (bc))
      case "shrink"
        bcfunc = @shrink_bc;

      case "discard"
        bcfunc = [];
        C -= length (Cpre);
        Cpre = Cpos = [];
        N = length (C);
        szx(dperm(1)) = N;


We have branching, function handle, and direct implementation on bc inside the
switch. If you are a potential developer coming to this function this is hell,
becuse is really hard to track the path of the function (the potential dev
might not want to become an expert of movfun). Why do it like this when this
can be avoided and become a single line? 

Separation of concerns also applies here, the mian function now is not only
dispatching but also implementing bc, why?

To add a new bc, in the current form,  one needs to edit at three points: the
cell with valid_bc, the switch part (making sure the if branching is not
introducing an block), and add the code of the subfunction. 
With the dispatch method, only the cell of valid_bc needs to be extended, and
the code of the subfunction added.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?48774>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/




reply via email to

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