Skip to content

Memory integration#5

Open
bleonar5 wants to merge 154 commits intomasterfrom
memory_integration
Open

Memory integration#5
bleonar5 wants to merge 154 commits intomasterfrom
memory_integration

Conversation

@bleonar5
Copy link
Owner

No description provided.

@bleonar5 bleonar5 requested a review from joshbenhamou October 27, 2021 13:15
Copy link
Collaborator

@joshbenhamou joshbenhamou left a comment

Choose a reason for hiding this comment

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

Can you share a little about what the goal of this MR is? It's hard to review this without context


public static function initializeMemoryTasks($group_id, $randomize,$final,$test) {
if($final){
$taskArray = '[
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this all stringified?

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is a major holdover from Gabe's design that I chose to integrate rather than rebuild. I think if I had to justify why he did it this way, it's because we have generalized tables for GroupTasks and IndividualTasks, all of which have different subtypes (for instance, cryptography vs memory), and each subtype has some parameters that don't overlap with the other subtypes. So I think in order to avoid a prohibitively wide table and to keep these parameters accessible in a generalized way, he stringified these arrays so he could store them in one column called "parameters"

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see okay, thanks for clarifying. Figured that this was a holdover but was curious about it

…ity (no longer interferes with active study to lock login page)
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