Skip to content

rectangles: add to track#243

Merged
jtigger merged 1 commit into
masterfrom
rectangles
Jan 18, 2017
Merged

rectangles: add to track#243
jtigger merged 1 commit into
masterfrom
rectangles

Conversation

@stkent
Copy link
Copy Markdown
Contributor

@stkent stkent commented Jan 10, 2017

@stkent
Copy link
Copy Markdown
Contributor Author

stkent commented Jan 15, 2017

I have tests done locally, still need to implement. FYI, I submitted this PR upstream since the final test case caught me a little off-guard.

final class RectangleCounter {

int countRectangles(final String[] rawGrid) {
System.out.println(Arrays.toString(rawGrid));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should remove.

for (int rightCol = leftCol + 1; rightCol < nCols; rightCol++) {

if (formsRectangle(topRow, bottomRow, leftCol, rightCol)) {
result++;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Inelegant but effective...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... which is just fine for the example... :)


private static final class Grid {

private enum Entry {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this enum overkill given how little it is used?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nah... I think it adds to the readability of the example code (yeah, while we are laxed about it, let's not go out of our way to avoid clean code!). I'm less excited about the name Entry... but I don't have a great suggestion right this second for a better term... Stroke?... I'll get back to ya on that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Element?


private boolean isHorizontalLineSegment(final int row, final int leftCol, final int rightCol) {
return stream(copyOfRange(getRow(row), leftCol, rightCol))
.allMatch(entry -> entry.equals(Entry.HLINE) || entry.equals(Entry.CORNER));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If the Entry enum stays, I could add an isHorizontalConnector method to wrap this functionality.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍


private boolean isVerticalLineSegment(final int col, final int topRow, final int bottomRow) {
return stream(copyOfRange(getCol(col), topRow, bottomRow))
.allMatch(entry -> entry.equals(Entry.VLINE) || entry.equals(Entry.CORNER));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If the Entry enum stays, I could add an isVerticalConnector method to wrap this functionality.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@stkent
Copy link
Copy Markdown
Contributor Author

stkent commented Jan 15, 2017

I miss Python's itertools: https://github.com/exercism/xpython/blob/master/exercises/rectangles/example.py#L99-L101

😂

@stkent stkent changed the title dibs: I will implement rectangles rectangles: add to track Jan 15, 2017
@stkent
Copy link
Copy Markdown
Contributor Author

stkent commented Jan 16, 2017

Upstream issue was resolved; I'll add new tests soon :)

@stkent
Copy link
Copy Markdown
Contributor Author

stkent commented Jan 18, 2017

New tests added, this is a-ok to pull for review 🔍

@jtigger jtigger self-assigned this Jan 18, 2017
Copy link
Copy Markdown
Contributor

@jtigger jtigger left a comment

Choose a reason for hiding this comment

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

I'm super excited about this exercise. I love that we're getting more of these that are just straight-up fun+challenging! Thank you for this, @stkent.

I think I had some lame nitpick about the name of the enum in the example code, but not enough to hold this work up. If you are so inspired, you can come back with a follow-up PR.

Looks beautiful. It's complete. A great add.

for (int rightCol = leftCol + 1; rightCol < nCols; rightCol++) {

if (formsRectangle(topRow, bottomRow, leftCol, rightCol)) {
result++;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... which is just fine for the example... :)


private static final class Grid {

private enum Entry {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nah... I think it adds to the readability of the example code (yeah, while we are laxed about it, let's not go out of our way to avoid clean code!). I'm less excited about the name Entry... but I don't have a great suggestion right this second for a better term... Stroke?... I'll get back to ya on that.


private boolean isHorizontalLineSegment(final int row, final int leftCol, final int rightCol) {
return stream(copyOfRange(getRow(row), leftCol, rightCol))
.allMatch(entry -> entry.equals(Entry.HLINE) || entry.equals(Entry.CORNER));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍


private boolean isVerticalLineSegment(final int col, final int topRow, final int bottomRow) {
return stream(copyOfRange(getCol(col), topRow, bottomRow))
.allMatch(entry -> entry.equals(Entry.VLINE) || entry.equals(Entry.CORNER));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Comment thread exercises/settings.gradle
include 'pig-latin'
include 'queen-attack'
include 'raindrops'
include 'rectangles'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💖

@jtigger jtigger merged commit 5422e53 into master Jan 18, 2017
@stkent stkent deleted the rectangles branch January 18, 2017 17:01
@stkent
Copy link
Copy Markdown
Contributor Author

stkent commented Jan 18, 2017

Thanks for the kind words, @jtigger! I'll follow-up with some polish in the next couple of days :)

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