Skip to content

Conversation

@dianavermilya
Copy link
Collaborator

No description provided.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a helpful variable name.

@dianavermilya
Copy link
Collaborator Author

Grade: 4.0 / 5
Great work. It looks like all your functions are up and running.
A few thoughts:

While your code is very clean and short, it could be a lot more readable. Use descriptive variable names so that other people (and you a few months later) can tell what is going on. So try to avoid 1 letter variable names as well as names like "temp". Code readability is VERY important, since most of the time you spend programming will be working with code that you didn't write yourself, not writing brand new code.

Most of these functions are working with codons, not DNA. Instead of manipulating the entire DNA string, it's a lot easier to build a list of strings, where each string is 3 base pairs. ["ATG", "GGA", etc. ]. This will save you a lot of multiplying and dividing by 3.

Once you've read the comments (both on the PR and under files changed), make a comment to the pull request or send me/Diana an email, letting us know that you've checked them out. Afterwards, close the PR (and optionally delete the graded branch), and we'll give you another 0.5 points.

REMEMBER NOT TO MERGE THESE PRs!

@ksuzy31
Copy link
Owner

ksuzy31 commented Feb 26, 2014

I have looked at the suggestions - kevin

@ksuzy31
Copy link
Owner

ksuzy31 commented Feb 26, 2014

thanks and although i didnt do this in hw4, i plan to follow the suggestions and make it more readable.

Thanks diana

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants