benknoble / comp401-a7 Goto Github PK

If people have a ConcurrentModificationException in their unregister methods, @benknoble 's testUnregisterROIObservers() in ObservablePictures.java will show up with the message

legal unregistration of multiple observers by region threw error: null

It's possible ConcurrentModificationException doesn't have a message which is causing the null (I don't know much about getMessage() to be honest)

Might have to add an extra catch for this error, specifically, with something like:

catch (ConcurrentModificationException e) { fail("legal unregistration of multiple observers by region threw error: " + e); }

instead of the e.getMessage()

@jersoe You're tests contain lines similar to

assertEquals("ROIObserver o1 didn't get called, but a pixel in it's region was changed.", o1.getLastRegion(),r1);

Which confuses me: if o1 is watching r1, but the region it needs to be notified of is just the one that contains c1 (0,0) which is (0,0)->(0,0). So instead, it should be different. From KMP's spec

if we call setPixel on P at position (1,1), then A will be notified once with the region (1,1)->(1,1)

Does this make sense?

In your test your seeing if unregisterROIObserver() throws an IllegalArgumentException if it's called with an observer parameter that isn't registered. Why? That's not a requirement as listed in the assignment. Would it be better to simply not do anything?

try { p.unregisterROIObserver(obsB); fail("didn't throw for unregistration of observer not registered"); } catch (IllegalArgumentException e) { }

Test File

It looks like the test is comparing the entire region being observed r1 with the region that is changed o1.getLastRegion() when it should be comparing the region of the one coordinate (0, 0) that changed as stated in the assignment instructions

So, for example, if we call setPixel on P at position (1,1), then A will be notified once with the region (1,1)->(1,1) as the second argument to the notification method

But I may be reading your TestROIObserverImpl.java file wrongly

I failed this test:
try { ROIObserver[] r = p.findROIObservers(tiny); assertTrue("observer not found", r != null && r.length == 1 && r[0] == obsB); } catch (Exception e) { fail("finding observers threw: " + e.getMessage()); } }
But I'm not sure if the test is correct.

There are 3 observers:
p.registerROIObserver(obsA, big); p.registerROIObserver(obsA, tiny); p.registerROIObserver(obsB, big);

Big covers the whole picture, from 0,0, to 3,3. Tiny covers an region from 2,2 to 3,3. We're asking findROIObservers to give us an array of all observers that are interested in the region tiny. Both are. Tiny falls within big, so if an observer is interested in big, then it's also interested in tiny. If it's interested in tiny, then it's interested in tiny.

Why do you think the method should not return obsA, and only obsB?

_findROIObservers(Region r)

This method returns an array of ROIObserver objects (possibly empty) that have registered regions of interest that intersect the given region "r". The same observer object may appear more than once in the result if it happens to be registered more than once with regions of interest that intersect the specified region._