Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed the nullpointer exception when cancelling batch grader #481

Merged
merged 16 commits into from
Mar 3, 2023

Conversation

VortiganOfficial
Copy link
Collaborator

Description

I couldn't find any particular place where the code was breaking other than the particular line that crashed, I tried putting return if null statements at certain points but nothing seemed o fix it alone, so I just started putting them anywhere that made sense

Closes #438

Type of change

  • Enhancement (improvement to an already existing feature)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • The app opens on my laptop

@VortiganOfficial VortiganOfficial changed the base branch from master to dev February 28, 2023 21:14
@VortiganOfficial VortiganOfficial changed the title Fixed the nullpointer exception when cencelling batch grader Fixed the nullpointer exception when cancelling batch grader Feb 28, 2023
Copy link
Collaborator

@Corppet Corppet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is not formatted correctly to our checkstyle standards. Additionally, randomly putting null-checks-and-returns within the code is not a good approach to solving this issue (consider try-catch instead).

@VortiganOfficial
Copy link
Collaborator Author

VortiganOfficial commented Feb 28, 2023

Sorry, there was a miscommunication, the placements were not random. I put them after the lines that opened the preferred directory, the folder containing the solutions, getting the selected file, and creating the result file. The reasoning for each placement is as follows:

I feel that there should be a return if null after opening the preferred directory because there is a default preferred directory and it being null is an error

I put a return if null after opening the folder because that solves the main problem, clicking cancel on the folder browser after clicking the batch grader button

I put a return if null after opening the selected file because no operations can be done on a file that is null, leading to possible bugs in the future

I put a return if null after creating the result file because the file might not get created if the program doesn't have write permissions in a certain folder, which is an error. I understand that this probably should be followed with an error message to the user, and I will open an issue for this as I don't know the specifics of how exactly to do that.

Lastly, I don't understand how I would implement try-catch blocks for this problem given my very limited experience with the language. If you have any more details or pointers to tutorials I will gladly follow them, but all tutorials I have found so far catch any and all exceptions, which is probably bad practice because the errors can be anything

I will fix the checkstyle errors, thank you for pointing those out to me. It compiled fine for me on my laptop, so I assumed it would be fine to create a pull request

EDIT: Upon examining other places where clicking cancel is handled gracefully, the same method is used in multiple other places

@Corppet
Copy link
Collaborator

Corppet commented Mar 3, 2023

I'll approve these changes since after looking at them again they're good enough to get the issue done.

For catching specific exceptions, simply specify the exception you want to catch in your catch block.

try
{
    // do something
}
catch (SpecificException see)
{
   // do something with se
}
catch (Exception e)
{
   // do something with e
}

@Corppet Corppet merged commit b348538 into Bram-Hub:dev Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Batch Grader throws exceptions if no file is selected.
3 participants