search

farhanrahman / kyoto

18 stars 7 forks source link

Added exception to CarbonAbsorptionHandler(), some people's code needs fixing #130

Closed n1kunj closed 12 years ago

n1kunj commented 12 years ago

As title. It now throws an exception if you don't have enough arable land, which needs to be caught and handled.

The exception used to be in there, but somebody thought it would be a good idea to remove it.

piotroslaw commented 12 years ago

I fixed my code.

CarbonReductionHandler functions should get a similar treatment, but after trying to change it I have noticed that it breaks Nik's code, so I left it as it is. Just something to keep in mind, as this can potentially break an agent's behaviour. I'm safe here, but could other behaviour writers make sure they don't use it in an unsafe way? (i.e., always check if the investment is possible in the first place)

n1kunj commented 12 years ago

I didn't change CarbonReductionHandler mostly because I couldn't be bothered, but also because it's a lot easier to sanity check that you're trying to reduce more carbon than you have than it is to check if the change in arable land caused when investing a given amount of carbon is more than you have.

piotroslaw commented 12 years ago

Agreed, we shouldn't change anything at this point if it works, just saying that if someone gets some weird behaviour and doesn't know why, this may be an issue

ChrisSSocha commented 12 years ago

Morning,

The exception used to be in there, but somebody thought it would be a good idea to remove it.

I feel like this is being blamed on me, as I went on a exception handling rampage. You can see the CarbonAbsorptionHandler file before my commit, here.

The Exception originally thrown by CarbonAbsorptionHandler() is due to its call to calculateOccupiedAreaMeasure(...), which threw an Exception. This was incorrect, as calculateOccupiedAreaMeasure(...) should throw a RuntimeException, which is an unchecked exception, and so the throw statements were removed from the methods. The error you discuss here never threw an exception, but you thought it did due to the bad exception handling everywhere else in the file.

Note that I raised this as a public issue #106, and made the changed known to those in college.