Skip to content
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

Refactoring exercises implementation #271

Closed
ErikSchierboom opened this issue Jun 14, 2016 · 6 comments
Closed

Refactoring exercises implementation #271

ErikSchierboom opened this issue Jun 14, 2016 · 6 comments

Comments

@ErikSchierboom
Copy link
Member

There are several refactoring exercises, including one on markdown rendering. How should the default implementation, which is to be refactored by the user, be done? Should it be a completely correct solution, or is it enough to only have the tests pass (and perhaps even deliberately only the tests)?

@ErikSchierboom
Copy link
Member Author

Anyone?

@Cohen-Carlisle
Copy link
Member

I don't think it has to be perfect by any means. But it should pass the tests. On the other hand, I don't think we should just make it pass the tests, but deliberately introduce a defect that would be caught if the appropriate test was added. Possible exception to the last sentence: include commented out tests that detect glitches in the current implementation that could be fixed on refactor.

@kytrinyx
Copy link
Member

If we're doing refactoring exercises, I would prefer to provide a test suite that intends to keep them safe. While it is important to be able to evaluate the test suite and decide that it's not good enough to refactor, I'd rather have the refactoring exercises start giving people the experience of what it feels like to refactor with a good test suite.

@petertseng
Copy link
Member

I'll offer you that of tree-building and ledger, both of which are currently only implemented by Go, the default implementations pass the tests but leave much room for improvement.

The default tree-building implementation has unhelpful error messages, duplicate code, perhaps a few too many levels of nesting, and probably more.

The default ledger implementation also has much repetitive code, a questionable usage of a channel, questionable variable names, and quite a lot more that I'm not getting into.

We can discuss whether this sort of thing is the model that should be followed for other tracks - these were certainly written before my time so I didn't have a chance to discuss it back then.

@ErikSchierboom
Copy link
Member Author

Okay, so I guess the strategy should be: the example implementation should be a valid implementation, not using hacks. It should only be of lower code quality. I'm fine with that.

@kytrinyx
Copy link
Member

the example implementation should be a valid implementation, not using hacks. It should only be of lower code quality.

Yeah, I think that's a good place to start. We can reevaluate later if someone comes up with good arguments for a different approach.

emcoding pushed a commit that referenced this issue Nov 19, 2018
…nd_Readme

Nil is not special objects in a circular buffer
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

No branches or pull requests

4 participants