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

Bridges.h incorrect throw documentation #30

Closed
dcarmer opened this issue Nov 9, 2015 · 10 comments
Closed

Bridges.h incorrect throw documentation #30

dcarmer opened this issue Nov 9, 2015 · 10 comments

Comments

@dcarmer
Copy link
Contributor

dcarmer commented Nov 9, 2015

Several functions in Bridges.h are documented as throwing exceptions, when they do not.

The documentation should not indicate that these functions throw.

@krs-world
Copy link
Contributor

I think I have fixed all of them.

@dcarmer
Copy link
Contributor Author

dcarmer commented Nov 16, 2015

setAssignment(int) is still documented as throwing an exception.
If the validation fails, bridges calls exit(EXIT_FAILURE), but this still doesn't throw.

@krs-world
Copy link
Contributor

I see a throw in the try block (for catching negative assignment numbers. Wouldnt that be caught in the catch block?

@dcarmer
Copy link
Contributor Author

dcarmer commented Nov 17, 2015

Yeah, exactly. The function throws internally, but also catches it internally, so for documentation purposes it shouldn't be declared as throwing an exception because it will always catch itself, without actually throwing to the user who is calling it.

And again I would recommend changing it like you have for the others. In that, since you already have a check for the throw, it would be better to put the catch calls in the if statement and leave the remainder in the else, without using a try/catch or manual throw altogether.

@krs-world
Copy link
Contributor

removed try/catch in setAssignment

@dcarmer
Copy link
Contributor Author

dcarmer commented Nov 17, 2015

Looks good, but it is printing the error message to cout.
This should be changed to cerr, to keep in line with similar situations.

@krs-world
Copy link
Contributor

fixed.

@dcarmer
Copy link
Contributor Author

dcarmer commented Nov 17, 2015

setAssignment still seems to be printing to cout not cerr. Additionally Bridges.h's server error msg and visualize functions are also printing to cout instead of cerr.

@krs-world
Copy link
Contributor

fixed.

@dcarmer
Copy link
Contributor Author

dcarmer commented Nov 19, 2015

visualize is still using cerr

Also since this is an if else list, I would exclude the explicit "None" check and just leave its code in an else statement at the end, that way if for some odd reason the visualizer type is not one of these types it will still be handled.

@dcarmer dcarmer closed this as completed Dec 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants