Hi! On Mon, Dec 17, 2012 at 2:42 PM, Jeff Davis <pg...@j-davis.com> wrote:
> I have attached a patch with some significant edits. > > * In your patch, there was still an inconsistency between the comment > for bounds_adjacent and the code. I refactored it to ensure it always > takes the upper bound as boundA, and the lower bound as boundB, so that > it can invert the inclusivities to create A..B to match the comments. > > * In the consistent method, you were inverting upper to be a lower bound > and lower to be an upper bound. I don't understand why (perhaps I did > the first time I read the patch), so I removed it. > > * It looks like the comments for which1/2 were inconsistent with the > code. I tried to fix that. > > * I significantly refactored the comments and code for the consistent > method. I had trouble understanding the original comments, particularly > around the edge cases. > > Please take a look and see if it still matches your algorithm properly. > This patch is not intended to be a final version (I didn't analyze my > code carefully), but just to show you what I mean and how I interpret > what the code is trying to do. You don't need to use my refactorings, > but if not, the comments in your version need more improvement so I can > understand. > > I found it easier to reason in terms of horizontal and vertical lines, > and which quadrants they crossed, and then work out the edge cases. I'm > not sure if that reasoning was correct, but it seemed to make more sense > to me. > Your comments and refactoring looks good for me. ------ With best regards, Alexander Korotkov.
