Skip to content

Utilizing Autobot for plank time recording #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Conversation

abMatGit
Copy link
Owner

[WIP] To get deploying

@abMatGit
Copy link
Owner Author

do not merge, this will require a lot of refactoring before I'm okay with merging.

@alan-andrade
Copy link
Collaborator

I can't help it but I think there's a lot of architecture here that might not be needed. I wonder if our code would become easier to read if we didn't worry too much about "adapter" pattern and so forth. An adapter could be a function, that reads nicely as a function. I feel the goal of the game is to have functions that read as what they're actually doing. slack_adapter = fucntion () {...}

@abMatGit
Copy link
Owner Author

abMatGit commented Oct 25, 2016

Yea you've touched on a couple things that have been irking me in this pull request:

  • How bloated certain classes are becoming
  • How these classes are losing their minimal, concise and simplistic purpose.
  • The lack of clarity on how these classes interact with one another. E S P E C I A L L Y as this project grows in complexity.

My thought process initially was: "What does an adapter do?"
I thought it would:

  • Parse the input specific from the source its adapting.
  • Feed that parsed input into the core.
  • Retrieve the result from the core.
  • Render the result from the core in a unique way back to the source.

To me, that described an object with behavior.

But even from implementing plankbot, these simple tasks are becoming more and more complicated. Take the parsing for example. Parsing slack input? This will include regex. This will include speech pattern recognition. This will also need to know how to adapt the recognized input text and map it to core methods. Rendering the response requires the same amount of research and code vice-versa.

And more to this, these are not things we can just confine into functions. These are classes, if not libraries that we will need to import to properly clean out this cruft.

I'm not entirely sure if I should halt progress because of these issues, because it will only get worse if I try to half-ass implement them.

@alan-andrade
Copy link
Collaborator

I don't think progress should be halted. We can do both things at the same time. While we add features, we could be refactoring for simplicity.

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