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

Added an api to accept block of code and to return result and time taken to execute. #3543

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

FuncGuy
Copy link

@FuncGuy FuncGuy commented Jul 30, 2019

Hi,
@kluever @kevinb9n @ogregoire
I've added an API to accept block of code and return result and time taken to execute. #3535

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@FuncGuy
Copy link
Author

FuncGuy commented Jul 30, 2019

@googlebot I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@FuncGuy FuncGuy changed the title Added api to accept block of code and return result and time taken to execute. Added an api to accept block of code and return result and time taken to execute. Jul 30, 2019
@FuncGuy FuncGuy changed the title Added an api to accept block of code and return result and time taken to execute. Added an api to accept block of code and to return result and time taken to execute. Jul 30, 2019
@kluever
Copy link
Member

kluever commented Jul 30, 2019

I think we (also) probably want an API that only returns the elapsed Duration, where the result of the method invocation can be ignored or is void.

@ogregoire
Copy link

I don't like how the exceptions are handled.

We do not know if there was an exception or not. There is currently no difference between an exception thrown by the measured code block and an exception being returned by the same code block.

I don't know what the people would prefer: let the exception pass through and get no result of a timed stopwatch, or on the contrary measure the code block and get the caught exception in a specific returned value.

So I'd say this would require more API:

  • measure -> Duration (ignore result, and let the exception through)
  • measureAndGet -> TimedResult (get the result, but let the exception through)
  • measureAndCatch? -> ExceptionallyTimedResult? (get the result, catch the exception, measure anyways)

Anyways, this should probably be better refined.

@FuncGuy
Copy link
Author

FuncGuy commented Jul 31, 2019

@kluever @ogregoire
I've addressed the review comments by @ogregoire if any further change is needed please mention it.

@netdpb
Copy link
Member

netdpb commented Jul 31, 2019

Looks relevant to #3528.

Comment on lines +284 to +285
public void update() {
}
Copy link

Choose a reason for hiding this comment

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

why an empty method, () -> update() can be replaced with () -> {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants