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

pascals-triangle: adjust API to use an instance method over class methods #365

Closed
jtigger opened this issue Mar 8, 2017 · 6 comments
Closed
Assignees
Labels

Comments

@jtigger
Copy link
Contributor

jtigger commented Mar 8, 2017

As per #177, adjust the test suite to require a solution that uses an instance method instead of a class method.

(see #344 as an example and the conversation about Function Object vs. Value Object)

@stkent
Copy link
Contributor

stkent commented Apr 5, 2017

Currently this exercises requires a single class that contains two methods with the following signatures:

public static int[][] computeTriangle(int)

public static boolean isTriangle(int[][])

Thoughts as to whether these should remain in a single class after conversion to instance methods? The responsibilities of each method are quite different, so I was thinking that two separate classes might be more appropriate here...

@stkent stkent self-assigned this Apr 5, 2017
@jtigger
Copy link
Contributor Author

jtigger commented Apr 7, 2017

I absolutely agree, Stuart.

computeTriangle() is a generator a representation of a Pascal's Triangle whereas isTriangle() is a validator.

Also, seems like the latter is dependent on the former (or I guess could be?). This is the first opportunity I've faced with considering dependency injection on the track. Don't worry, I'm not talking about depending on Dagger or Spring or anything... :)

The question in my mind is whether the TriangleValidator should depend on a TriangleGenerator?

...

Also... until now, I'm pretty sure we haven't required the practitioner to submit more than one file. Should we should include a HINT.md to "remind" them of that feature of the CLI exists?

@stkent
Copy link
Contributor

stkent commented Apr 7, 2017

Dagger or Spring

😰 😂

The question in my mind is whether the TriangleValidator should depend on a TriangleGenerator?

Hmm... I think it's reasonable here. I'm assuming you're thinking of constructor injection - that would seem most natural to me based on how we'd specify the tests. And yes, if we go this route, a HINT.md file is definitely in order. In that case I'd also like to use this as an opportunity to begin documenting these sorts of policies in a single centralized location so that it's easier to regularly review the track for compliance with said policies. In this case, the policy statement would be something like:

The first exercise in the track whose test suite mandates multiple solution files should be accompanied by a HINT.md file reminding the practitioner that the CLI supports multiple file submissions.

and the description of situations in which we would want to double-check the application of this policy would be:

Any time the track order changes/any time the number of solution files required by an exercise test suite changes.

[If these events weren't so infrequent, I'd suggest that we could build this checklist into a pull request template, but I think it would add too much noise.]

@stkent
Copy link
Contributor

stkent commented Apr 7, 2017

tl;dr: I'll have a crack at all of the above and we can then review and see how it feels!

@jtigger
Copy link
Contributor Author

jtigger commented Apr 14, 2017

@stkent said:

In that case I'd also like to use this as an opportunity to begin documenting these sorts of policies in a single centralized location so that it's easier to regularly review the track for compliance with said policies. In this case, the policy statement would be something like:

The first exercise in the track whose test suite mandates multiple solution files should be accompanied by a HINT.md file reminding the practitioner that the CLI supports multiple file submissions.

I couldn't agree more on both the notion of (finally) standing-up a policy page (perhaps organized by triggering event) and the way you've articulated this particular one.

... and then ...

[If these events weren't so infrequent, I'd suggest that we could build this checklist into a pull request template, but I think it would add too much noise.]

Yup, I totally agree with you, there. Applying these policies seems like something someone with "maintainer's glasses" on.

@stkent
Copy link
Contributor

stkent commented Apr 14, 2017

Turns out the canonical data removes the validation steps from this exercise! I updated the implementation to match while addressing this issue. I'll make a new issue to kick off policy tracking!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants