CMPUT301W15T12 / C301Project

Apache License 2.0
3 stars 2 forks source link

Updated implementation of setCategory and setCurrency #68

Closed IchigoKIl closed 9 years ago

IchigoKIl commented 9 years ago

I have changed their implementation, as discussed in the last group meeting, to actually check if categories/currencies are valid. The tests US04.02.01 and US04.03.01 have been updated to utilize this change.

It appears no extra changes will be necessary to conform with this update and I have rerun the junit tests without any issues. If there are any issues please let me know.

vanbelle commented 9 years ago

These exception don't get thrown in the activities beacuse you chose to make them runtime exceptions. They also don't allow for unknown/incomplete expense items.

Can we add an Unknown setting in these currencies/categories?

IchigoKIl commented 9 years ago

What is the problem with using a runtime exception? As of now the app crashes when an expense with an invalid category/currency is created, as expected.

As for an unknown setting, instead of doing that I will just make it check if the argument is null, if it is then it will skip the check and set the category/currency to null as desired.

vanbelle commented 9 years ago

The problem is we don't want the app to crash. We want the exception to be thrown in the activity code so that we can force the user to choose the appropriate setting., and runtime exceptions don't get thrown. They don't give the chance to fix anything, they just crash the program. Our program should never crash.

On Mar 31, 2015, at 10:24 PM, IchigoKIl notifications@github.com wrote:

What is the problem with using a runtime exception? I tried testing this by adding a line to create a new expense item when the login button is clicked, and the app crashes as expected. As for an unknown setting, instead of doing that I will just make it check if the argument is null, if it is then it will skip the check and set the category/currency to null as desired.

— Reply to this email directly or view it on GitHub.

IchigoKIl commented 9 years ago

It won't crash if you put the expenseitem creation in a try catch block, although the exception shouldn't occur since we have spinners with valid inputs. I tested this code in the on click of the login button: try { ExpenseItem expenseItem = new ExpenseItem(null, null, null, "AD", null, null, true); } catch (Exception e) { Toast.makeText(LoginActivity.this, "crash", Toast.LENGTH_SHORT).show(); }

and it shows the toast as expected. However, if you have a different type of exception you want to use then I will change it to that, but will probably have to add throws exception in the function declaration in several places. This was just the easiest way to do this with minimal changes to the code.

vanbelle commented 9 years ago

Ok, it was just causing some problems and at the time I was having trouble figuring out where the error was coming from. The problem was stemming from the fact that we never pass a null value, we were passing "" when it was incomplete, so i had to change that in your code, but everything works now.