search

stepthom / math-fun

Just testing out some fun with mathematics.
2 stars 2 forks source link

Issue #58 update: equals() for MathFunction #61

Closed johns1342 closed 10 years ago

johns1342 commented 10 years ago

Added an equals() function and a generic hashCode() function to the MathFunction class, (both were basically auto-generated from the Eclipse tools, I just changed the format a bit to line up with the Term equals function). Did some quick testing and it appears to work since the termsByExponent TreeMap container is essentially calling equals on all keys and values (using Set equals). Should I write a unit test for it before this is reviewed?

stepthom commented 10 years ago

@johns1342 Could you please also add some new unit tests to ensure this function is behaving as desired?

johns1342 commented 10 years ago

Sorry, that last push went horribly wrong. I just created a new branch from this version to update the equals and hash functions, and was trying to push that to GitHub, but it also pushed the latest version as well to this update, instead of just that branch. So it's a big mess now because the code shown has partial updates for another issue. I'm not sure how to go about correcting it yet, so that this pull request is only the equals and hash functions of MathFunction.java, and eventually a unit test to validate it. Is the problem that I initiated a pull request from my master, instead of the branch?

If that's the case, for this issue, is it easier to just cancel this pull request and start another from my Issue #58 branch?

stepthom commented 10 years ago

@johns1342 Yes, it looks like you issued this pull request from your master branch, so any other commits you make to that branch will be shown on this pull request. You have two options to fix it:

Do those make sense?

johns1342 commented 10 years ago

@stepthom Yes, makes sense, thanks! I'm going to close this PR, get the branches sorted out, finish up the unit test, and then submit again. Definitely a good learning experience. For workflow, do you typically keep your own main clean and try to only import changes from the 'main' master, or do you merge your local branches back to main? It seems like the second way could cause some merging issues?

On Tue, Dec 17, 2013 at 8:18 AM, Stephen W. Thomas <notifications@github.com

wrote:

@johns1342 https://github.com/johns1342 Yes, it looks like you issued this pull request from your master branch, so any other commits you make to that branch will be shown on this pull request. You have two options to fix it:

  • Undo your last commit (638fc01https://github.com/stepthom/math-fun/commit/638fc01) to your master branch, and then keep working on this issue on your master branch
  • Close this PR. Create a new branch just for this issue, apply your original commit (22aac4chttps://github.com/stepthom/math-fun/commit/22aac4c9b6b136003899401d9fa8c6b8903de6a4) to the new branch, and create a new PR for that branch.

Do those make sense?

— Reply to this email directly or view it on GitHubhttps://github.com/stepthom/math-fun/pull/61#issuecomment-30749704 .

stepthom commented 10 years ago

@johns1342 To answer your question: I normally only work in a single repository, so I don't have a lot of experience merging branches from forked copies of the repo, as you are doing. In my single repository, I follow the convention that all changes happen to temporary branches, which get merged into master when they're ready, and then deleted.