Added _394.java #31

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

Jump to bottom

Merged

fishercoder1534 merged 1 commit into fishercoder1534:master from varunu28:master

Jan 11, 2019

Merged

Added _394.java #31

fishercoder1534 merged 1 commit into fishercoder1534:master from varunu28:master

Jan 11, 2019

Conversation

varunu28

Copy link

Contributor

@varunu28 varunu28 commented Jan 8, 2019

Added code with description
Added test case
Updated README

@fishercoder1534

Copy link

Owner

fishercoder1534 commented Jan 8, 2019

Left some comments, LGTM otherwise, thanks!

@varunu28

Copy link

Contributor Author

varunu28 commented Jan 8, 2019

@fishercoder1534 I am unable to view your comments on the PR and it is showing me no reviews against the PR also. I was facing this issue during my last PR too. Not sure if Github changed something.

@varunu28

Copy link

Contributor Author

varunu28 commented Jan 10, 2019

@fishercoder1534 Were you able to check on this?

@fishercoder1534

Copy link

Owner

fishercoder1534 commented Jan 10, 2019

Yes, I looked into it a bit, I was able to clearly view everything.
But when I switched into incognito mode, I don't see the comments, so I suspect it's due to permission issue, and I didn't really see how Github provides a way around this or setting on this repo that I could adjust.

fishercoder1534

fishercoder1534 requested changes

Jan 10, 2019

View reviewed changes

Copy link

Owner

@fishercoder1534 fishercoder1534 left a comment

Choose a reason for hiding this comment

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

Are you able to view comments this way? @varunu28

src/main/java/com/fishercoder/solutions/_394.java Outdated

public class _394 {

public static class Solution {

Copy link

Owner

@fishercoder1534 fishercoder1534 Jan 8, 2019

Choose a reason for hiding this comment

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

Nit: Solution -> Solution1 that's our convention to allow multiple solutions for one problem. Thanks!

Copy link

Contributor Author

@varunu28 varunu28 Jan 10, 2019 •

edited

Loading

Choose a reason for hiding this comment

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

Does a solution exist for this solution in the repo?

Copy link

Owner

@fishercoder1534 fishercoder1534 Jan 10, 2019

Choose a reason for hiding this comment

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

No, it's unique. It's not a namespace conflict thing, we usually start with Solution1, although there might be the only solution for that problem at the moment.
If you could stick to this convention, that'd be great.

Copy link

Contributor Author

@varunu28 varunu28 Jan 11, 2019 •

edited

Loading

Choose a reason for hiding this comment

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

Sounds great. Will make the changes

src/main/java/com/fishercoder/solutions/_394.java Outdated

count.push(Integer.parseInt(s.substring(start, idx + 1)));

}

else if (s.charAt(idx) == '[') {

Copy link

Owner

@fishercoder1534 fishercoder1534 Jan 8, 2019

Choose a reason for hiding this comment

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

Nit: move else if line to right behind }, otherwise build will fail like last time. see this commit and this.

src/main/java/com/fishercoder/solutions/_394.java Outdated

else if (s.charAt(idx) == '[') {

str.push("");

}

else if (s.charAt(idx) == ']') {

Copy link

Owner

@fishercoder1534 fishercoder1534 Jan 8, 2019

Choose a reason for hiding this comment

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

Same here.

src/main/java/com/fishercoder/solutions/_394.java Outdated

str.push(str.pop() + sb.toString());

}

else {

Copy link

Owner

@fishercoder1534 fishercoder1534 Jan 8, 2019

Choose a reason for hiding this comment

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

Ditto.

Copy link

Contributor Author

@varunu28 varunu28 Jan 11, 2019

Choose a reason for hiding this comment

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

Done

@fishercoder1534

Copy link

Owner

fishercoder1534 commented Jan 10, 2019

Looks like so, I think the problem was: I needed to use "Request Changes" and then click "Submit" instead of just commenting on the code. Lessons learned, fun!