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

Support for Exception handling #970

Conversation

abdelr
Copy link
Member

@abdelr abdelr commented Apr 27, 2021

No description provided.

@abdelr abdelr requested review from msevestre and PavelBal April 27, 2021 09:21
@abdelr abdelr self-assigned this Apr 27, 2021
@abdelr abdelr linked an issue Apr 27, 2021 that may be closed by this pull request
Copy link
Member

@PavelBal PavelBal left a comment

Choose a reason for hiding this comment

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

The API is convenient to use from R.

e.Message
);
}
return new ConcurrencyManagerResult<TResult>
Copy link
Member

Choose a reason for hiding this comment

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

why don't you return this from the try?
Also can you not name the parameter args
Last, don't hesitate to name parameters instead of passing false, or true => success: true

{
Id = id;
Succeeded = succeeded;
ErrorMessage = error;
Copy link
Member

Choose a reason for hiding this comment

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

Either call the parameter errorMessage or the Member Error. No reason to have different names here.

(
id(args),
true,
simulationResult,
Copy link
Member

Choose a reason for hiding this comment

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

It feels like two constructor. One with Id(artgs) results and there other one just with the id(args)and error instead of asking the caller to pass the right parameters. (e.g. "", false or null true etc..)

@abdelr abdelr requested a review from msevestre April 28, 2021 07:02
Copy link
Member

@msevestre msevestre left a comment

Choose a reason for hiding this comment

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

awesome 💪

@msevestre msevestre merged commit 895594e into develop Apr 28, 2021
@msevestre msevestre deleted the 968_ConcurrentSimulationRunner_Error_when_one_simulation_fails branch April 28, 2021 16:37
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.

ConcurrentSimulationRunner: Error when one simulation fails
3 participants