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

Tagged save numbers #176

Merged
merged 2 commits into from Jan 19, 2018

Conversation

Projects
None yet
2 participants
@PragTob
Owner

PragTob commented Jan 18, 2018

Not 100% happy with this implementation. Seems overcomplicated.
I changed the requirements a bit so that we don't create tag-1
but just tag and tag-2. Otherwise it'd be even more involved.

I considered making it more structure that a Tag is a structure
with %Tag{name: "foo", count: 1} or something like this.
But that also seemed like a bit of overkill.

So yeah, if you have anything simpler let me know. Otherwise
let me know if you think this is good enough :)

Attention 2 commits, first one is the implementation second one is mix.formating it. (better reviewability)

PragTob added some commits Jan 15, 2018

Tagged Save - implementation to avoid duplication
Not 100% happy with this implementation. Seems overcomplicated.
I changed the requirements a bit so that we don't create tag-1
but just tag and tag-2. Otherwise it'd be even more involved.

I considered making it more structure that a Tag is a structure
with `%Tag{name: "foo", count: 1}` or something like this.
But that also seemed like a bit of overkill.

So yeah, if you have anything simpler let me know. Otherwise
let me know if you think this is good enough :)
def write({term_binary, filename}) do
FileCreation.ensure_directory_exists(filename)
return_value = File.write(filename, term_binary)
IO.puts "Suite saved in external term format at #{filename}"
IO.puts("Suite saved in external term format at #{filename}")

This comment has been minimized.

@PragTob

PragTob Jan 18, 2018

Owner

I feel like I'd need to draw a line here....

@devonestes

This doesn't seem all that complex to me? I mean, it's simple and slightly on the verbose side, but I figured out what was going on here super easily.

We know it works thanks to the great tests, and if something more "elegant" pops into your head later, we can always go back and refactor.

@PragTob

This comment has been minimized.

Owner

PragTob commented Jan 19, 2018

Thanks for the feedback @devonestes ... dunno. It feels like I'm doing something very simple but I need ~30 lines of code to express what I'm doing. Feels too much. Feels like I'm somehow missing a more elegant solution, might just be due to the de-structuring of a string into tis parts though

Often code leaves me happy after I wrote it. This didn't. Felt clumsy.

Anyhow, Ghost isn't clumsy at all. 🎉

img_20171221_144657_bokeh

🐇

@PragTob PragTob merged commit e0e9891 into master Jan 19, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 94.931%
Details

@PragTob PragTob deleted the tagged-save-numbers branch Jan 19, 2018

@devonestes

This comment has been minimized.

Collaborator

devonestes commented Jan 19, 2018

Any time you're dealing with parsing actual meaning from a string, that starts to get gnarly pretty quick. I know it sort of feels like it should be easy because it's so easy for us humans, but that's one of the things computers unfortunately still are terrible at.

Some of the worst code I've ever written is all around trying to do some sort of natural language processing, or normalizing user input. Language is nasty stuff.

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