Skip to content

Conversation

KacieAhmed
Copy link

I applied the builder pattern in order to ensure no methods ever accidentally edit the genome. To do this, created a builder that builds the genome as an immutable object. Previously, cc built the genome, but it was mutable and the variable name wasn't informative. Now "GenomeBuilder" builds the genome.

  • Added genome.py (builder class)
  • Modified library to get rid of "cc" (previously cc built the genome)
  • Modified opt and corona to import the builder instead of cc.

@KacieAhmed KacieAhmed changed the title Pattern Builder Pattern Applied Apr 16, 2021
@Lajellu
Copy link

Lajellu commented Apr 18, 2021

Hi Kacie, thank you so much for your hard work and choice of this project to contribute to. Here is some feedback for your pull request:

Correctness of Creational Implementation

Points in Favour: This change does indeed belong in the codebase, and not in an external library because it is specific to the class and libraries like SimTk already have non-specific versions implemented for generic use.

Points Against: There are no comments in the Genome class to explain the reasoning for adding the builder (ie. making its genome immutable)

Choice of Creational Implementation

Points in Favour: The builder was partially an appropriate choice, because as mentioned in the paper, if a method were to change the data in the genome, it would represent a mutated version instead of the version we wish to target. The builder pattern maintain a consistent object state.

Points Against: Currently, there is no multithreading being used in this program, so protecting against changes in other threads is not yet necessarily. Therefore, it would be better to merge this pull request once threading is implemented.

Correctness of Creational Logic

Points in Favour: It is correct to have used the lib.py file as it is where the original codebase included “cc” to store the nucleotides list. The new code replaces this with a genome
object that is immutable, and named more appropriately.

Points Against: Perhaps another method could be added to the Genome class to extend functionality for the future, but this is not currently needed.

Clarity of Creational Logic

Points in Favour: This added code does not create an exorbitant level complexity, as it is easy to read, and coders are not likely to introduce bugs while trying to run or extend this code.

Points Against: A longer explanation could have been provided as to how the builder pattern makes the genome data thread-safe, so that other coders would be able to more clearly understand the logic behind the choice

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.

2 participants