15

Your code is fine and readable from a C/Java perspective. I don't think it's particularly Ruby-ic to use return statements. Just put list at the end on its own. Why your case is slow You have this extra puts here: str.scan(/A|C|G|T/) do |sub| puts case sub ^^^^ when "A" ... You may want to get rid of that :) That's why you interpreter prints ...


13

Perhaps there aren't enough test cases here, because I don't see why you can't just use: pattern = re.compile(r"[^ACGT]") print(pattern.sub("", toy_sequence)) Which says replace all characters that are not in ACGT with blanks. Thus, you do not need to know which characters are illegal, only those which are legal. This should be very fast,...


11

I am no C programmer but there are a few things that come to my eye: Static size You always read at most 1000 characters, what if the file size is much bigger? Instead you would loop trough the file, reading/processing it chunkwise. Magic Number Even if you use the static size you should not enter it at multiple locations. Instead use a constant for it ...


11

Well one simple change will make it do lines in groups of four. while (getline(std::cin, defline) && getline(std::cin, seq) && getline(std::cin, buffer) && getline(std::cin, qual) ) { // Only enter the loop if you correctly read all four lines. } We can also improve the test for a specific character. Note I am ...


11

In order to speed this up, you'll need to avoid as many string creation operations as possible, because they are expensive. Especially the split operation is expensive. Not only does this create many new strings, it does this mostly unnecessarily, because you don't need all the substrings. Instead you need to do some low level searching using only positions ...


11

An array of char* is not a great data structure for storing AA or codon sequence data. A pointer takes 4 or 8 bytes per codon, but there are only 64 possible codons so you could easily store one codon per byte as a uint8_t. And with only about 22 amino acids, that's close to being able to fit 2 AAs per byte, but it doesn't quite work and it's definitely ...


10

I'm just going to look at cond_probs here. You'll see that there's plenty in this function for one answer. 1. Comments on your code The line return probs has the wrong indentation. Copy/paste error? There are no docstrings. What do these functions do and how do I call them? (The text from your question would be a good starting-point for docstrings.) There ...


10

static std::string&& dna_to_upper_case(std::string& unpacked_dna_sequence). That's wrong. It returns a dangling reference. You can't return an rvalue reference to a temporary object. Return it by value. It doesn't change the unpacked_dna_sequence, so it would be better to pass it by a const reference. The same is true for std::string&& ...


10

If you are in for raw performance, try to avoid repeating potentially cost-intensive operations. In this case, you split the lines twice with the same parameter, which repeatedly applies a regular expression under the hood. Instead of Sring rName1 = line.split("\t")[2]; String rName2 = line2.split("\t")[2]; Stream<String> s1 = Stream.of(line.split("\...


10

Add functions Everything is currently in the global namespace, you can add a few functions to split up the code and make it more readable. You could use a count_occurence function and a write_output function. Avoid working with global variables, it can make the code less readable, secondly these are not really global variables. Make use of a dictionary, ...


9

This covers an interesting topic. Great work! Because I am unfamiliar with this area, I utilized your unit testing to ensure changes I make did not break functionality. If they do, I apologize and please let me know. Utilize implicit repetition and iota Rather than manually defining the type and value of nCode, aCode, etc. we an implicitly get the value ...


8

I've make small research on your question. My first idea was: def countACGT(str) str.chars.sort.join.scan(/A+|C+|G+|T+/).map(&:length) end Then I've write few another methods (Benchmark used to asses their performance). Also include method proposed by Barry. require 'benchmark' REP_NUM = 10_000 # number of tests runs. PATTERN = "..." # here is ...


8

You need a faster search method You are comparing every entry in duplexDict directly with every entry in duplexDict. This means the number of operations will increase with the square of the number of entries in duplexDict. This stands out from the lines: for i in duplexDict: ... for j in duplexDict: More formally, you algorithm runs in ...


8

First things first: Undefined behavior Kraskevich pointed at the most obvious one. Yet, there is one more (although it will work on X86, from what I've been told). Basically, do not pass chars directly to any function in cctype, convert them to unsigned char first. More info. vector bool Very interesting decision. I'd rather pack it myself using ...


8

Welcome to Code Review! I'll add to the other answer from Reinderien. PEP-8 In python, it is common (and recommended) to follow the PEP-8 style guide for writing clean, maintainable and consistent code. Functions and variables should be named in a lower_snake_case, classes as UpperCamelCase, and constants as UPPER_SNAKE_CASE. No unnecessary whitespaces are ...


8

There's an inconsistency between the two functions. check_and_clean_sequence() has an alphabet parameter, but this isn't used by the inner function, which has a hard-coded list of invalid characters to remove. The check_and_clean_sequence() function doesn't add value - all it does is add overhead of an extra pass through the string (re.sub() will just do a ...


7

Some notes: As others have already pointed out, you should use a hash instead of a gigantic case. But make sure your get operations on that hash are O(1), otherwise the method will be very inefficient. You can use Enumerable#take_while to manage the stop amino acids. Encapsulate the code in a module/class. You need a return because it's not the last ...


7

I don't really like self.sequence, it's only used in count_nucleobases, and doesn't seem to be needed as an attribute to the class. I'm not a fan of raise SystemExit. First if I'm reading things correctly, it causes the program to have an exit status of 0. Which means that it ran without errors, which is not true here. Secondly, I think of it more as ...


7

The Interface class packed_dna_sequence { public: packed_dna_sequence(std::string& dna_sequence); std::string&& unpack() const; std::vector<bool> get_packed_sequence() const; private: std::vector<bool> m_packed_bits; }; Does not look very useful. You have a full dna_sequence as a string already (so you have the space)...


7

The line (*aminoacid_string) = malloc(aminoacid_count); allocates aminoacid_count of bytes. The code needs that many pointers. The correct way is to *aminoacid_string = malloc(aminoacid_count * sizeof(**aminoacid_string)); Testing for the correct input (i.e. calling check_nucleobase_string) belongs to main. I recommend to translate the nucleobase ...


7

You should try to refactor the code to have less hard-coded constants. E.g. nucleobase_to_aminoacid has both tcag and codon_table hard-coded. That is in general something that hinders re-use. You should preferably have a separate function to print the codon_table - in a standard format. This would have detected that you used "Len" instead of "Leu" as ...


7

Path? Surely path is not a single path, since you loop through it. So at the least, this is poorly-named and should be paths. The variable you iterate through it, fileName, should be file_name; similarly for your other function and variable names. Line iteration Rather than calling readlines(), simply iterate over file itself, which will have the same effect....


7

From the bioinformatics side, not the python one: Your return will be non-useful for further processing whenever an ambiguous base has been present, because it changes index locations! You'll want to fix that before worrying about how long it takes to run... DNA and RNA are themselves a form of coding language: three base units ('codons') describe amino ...


6

The Rosalind problem takes in a string and counts the occurrences of A,C,G,T - reading the string from the file is not part of it so I shall skip over that. In terms of correctness - does what is intended - it seems fine. In terms of performance, as @Nobody mentioned, it is most likely that strlen() is being called multiple times, so doing it once ...


6

Style Guides The Perl community does not have a single universal style – there is more than one way to do it. But sometimes, consistency isn't a bad thing either. Here are the three important cornerstones of Perl style: A perlstyle manpage exists which explains a sensible core of a style guide. Damian Conway published the Perl Best Practices book with many ...


6

I have a limited understanding of the different parts of your code so I'll just comment on some details that can be greatly improved. General: A few things are not quite pythonic. You can make your code go through some automatic checks with pep8online (PEP8 is the Style Guide for Python Code) and pylint. About protease : def protease(PrEST_seq, a, type): ...


6

Your code looks pretty good to me, though admittedly I have a hard time understanding immediately what it's doing. I'll go through line by line and see if that helps my understanding. Main.java searchguicompare is a pretty awkward package name, but that's a trivial point. Your class searchGUIcompare should be called either SearchGUICompare or ...


6

There are a number of issues with this program. I'll start with some trivial ones first. Hard-coding the input filenames makes it inflexible throwaway code. I suggest taking command-line parameters (sys.argv). Variable names tmp and var are horribly uninformative. Also, Lina violates PEP 8 capitalization guidelines. PEP 8 also calls for four spaces of ...


6

Various comments in random-ish order. Main guard It is common to put your code "actually doing something" behind an if __name__ == "__main__": test. Data over code Because of many different things, your code contains a small amount of data diluated in a large amount of text. Among the different useful techniques to make things easier to maintain is to ...


6

Generally, these programming challenges don't require you to do validation. How strict do you make the validation? Why not verify that the string contains nothing but ACTGU? In any case, your validation framework seems like overkill for a simple length check. For the Hamming distance calculation itself, I suggest a more idiomatic one-liner left.chars.zip(...


Only top voted, non community-wiki answers of a minimum length are eligible