jeburke/RNA-is-awesome — pull request #1 by tipabu (2014-01-27 18:15:09+00:00)
Title: Fastq filter

Right, I meant to send you my ideas for changes. You could still do them yourself, of course, but I thought you might appreciate the feedback.

* I didn't see much point in `rstrip()`ing `line` just to tack on the end-of-line again on print.

* Since `out` is just being used as a flag for whether to print the record or not, may as well switch it to be a boolean. Doesn't matter too much here, but for larger projects I could see two potential issues:

* String comparisons are more expensive than simply checking truth-values. Granted, it'd have to be a pretty heavily-used piece of code to have a noticeable impact, but may as well avoid it.

* There may be issues with using "yes"/"no" in one area but "true"/"false" or even just "YES"/"NO" in another, possibly written by someone else. Integrating between them becomes problematic, especially if you aren't sure which value is the more important one.

(On a related note, I like that you consistently compared `out` against `"yes"`. I've definitely seen code that would do the equivalent of comparing against `"yes"` in one section, `"no"` in another, `"Yes"` in yet another, etc. As a result, the code didn't actually cover all the cases the programmer thought it did.)

* I pulled TDNA out of the loop, since we don't really need to change it. We can always just take slices to get what we want out of it. You might also want to consider passing it as an argument, either directly or as a one-line file containing the string to match against.

Also worth noting: the initial version of my inner loop was something like
``` python
for i in reversed(range(len(TDNA))):
if line[:i] == TDNA[:i]:
...
```
until I thought again about string comparisons. It varies a bit from language to language _exactly_ how it's implemented, but at some point it always needs to just start walking through the strings doing a character-by-character comparison. Doing it the way that I had, it'd keep doing that over and over, stopping at the same point, until I shortened the string enough. So instead I just did what the string comparison would have done.