Sorry guys, I have been busy with a few other things before continuing
on this one (also solving the pesky issue with my RubyInline install).
@Robert.
I have been thinking hard about your comments - these contain a lot of
programming insight of the kind you dont get from a "learnings
<programming language> book". However, I struggle with grasping the
wisdom:
"Frankly, I find your code has a design issue: it seems you mix data
and iteration in a single class. This is visible from how #match
works
def match(pattern, pos = 0, max_edit_distance = 0)
@pattern = pattern
@pos = pos
@max_edit_distance = max_edit_distance
@vector = vector_init
..
IMHO it would be better to separate representation of the sequence and
the matching process. The matcher then would only carry a reference
to the sequence and all the data it needs to do matching.
"
What is this "mixing data with iteration in a single class"? To me you
have data and then you iterate - what exactly is the problem? What
should I do for separating these?
I am referring to
http://pastie.org/1808127 : The issue with the code
presented lies in the *state*: an instance of Seq represents a sequence
of items (in your case amino acids, I guess). You need state to
represent this sequence. This state is stored in instance variables of
an instance of class Seq.
The first thing that your method #match (see above) does, is to set some
other instance variables of Seq. This poses a problem if
- the Seq instance is frozen, i.e. immutable,
- more than one matching processes are under way concurrently (i.e.
#match is invoked from more than one thread at a time).
The reason is that you mixed state needed to represent your sequence
with state needed to execute the matching process. Apart from the
problems listed above this also makes code harder to read and more error
prone. For example, you might modify the Seq implementation and
accidentally reuse the name of an instance variable which then can make
your code break in unpredictable ways.
"Maybe on the interface, but you create side effects on the String (Seq
in your case). This is neither a clean separation (makes classes
bigger and harder to understand) nor is it thread safe (e.g. if you
want to try to concurrently match several patterns against the same
sequence)."
Again, "separation", but what is the problem with big classes? When is a
class too big? And how to divide your code in the best way?
There is no easy answer to that. In this case you violate modularity by
lumping too many different functionalities together in a single class.
Apart from the advantage to avoid mischief laid out above by decoupling
the matching process from the sequence representation you also gain
modularity if you let the matching process only rely on the public
interface of Seq. That way you can even have different implementations
of sequences which can all be scanned by the same representation. I am
not saying you should necessarily do this as efficient matching might
also need some knowledge of Seq internals, but this is to illustrate
what kind of things you should consider when designing classes.
Also, I managed to get down to a single vector, but I think I may have
more .dup's than needed - though removing any causes erroneous output:
http://pastie.org/1902844
That code still stores matching state in the Seq instance. And you have
quite a few #dups which have object creation overhead. I think you
should better work with two Array of Score instances and swap them at
each sequence position. IMHO you do not even need to reinitialize Score
instances because of your dynamic programming approach it is guaranteed
that you access instances sequentially from index 0 on and need to
access at most last[i - 1], last
and current[i - 1].
Kind regards
robert
and my previously