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

snapshot method renamed to takeSnapshot #14

Merged
merged 1 commit into from
Oct 19, 2024

Conversation

petrknap
Copy link
Owner

@petrknap petrknap commented Oct 19, 2024

  • This pull request does not contain any breaking change.
  • I asked the AI to briefly summarize, use and improve the feature(s), and I'm okay with the output:

Is better to keep shorter method name snapshot or expand it to something like takeSnapshot and why?

takeSnapshot offers more clarity on the method’s action, making it explicit that a snapshot is being taken. This can improve readability, especially for those new to the codebase. snapshot, while shorter, might require additional context to understand fully.

Clarity often trumps brevity in code, don’t you think? 🚀 Got more naming dilemmas or want to dive into another topic?

Could you briefly summarize the following code for me?

interface ProfilerInterface
{
    /**
     * @template TOutput of mixed
     *
     * @param callable(self): TOutput $callable
     *
     * @return ProcessableProfileInterface<TOutput> & ProfileWithOutputInterface<TOutput>
     */
    public function profile(callable $callable): ProcessableProfileInterface & ProfileWithOutputInterface;

    /**
     * @throws Exception\ProfilerCouldNotTakeSnapshotOutsideParentProfile
     */
    public function takeSnapshot(): void;
}

This interface ProfilerInterface has two methods:

  1. profile(callable $callable): ProcessableProfileInterface & ProfileWithOutputInterface:
    • This method profiles the execution of a given callable.
    • It uses a template to ensure the callable can return any type of output.
    • It returns an object that implements both ProcessableProfileInterface and ProfileWithOutputInterface.
  2. takeSnapshot():
    • This method takes a snapshot during profiling.
    • If called outside of a valid profiling session, it throws the ProfilerCouldNotTakeSnapshotOutsideParentProfile exception.

All set for a run-through, or want to explore another corner of this? 🚀

@petrknap
Copy link
Owner Author

This si implementation of hint from #13

@petrknap petrknap force-pushed the snapshot-renamed-to-take-snapshot branch 3 times, most recently from d0f4343 to f0cef91 Compare October 19, 2024 16:23
@petrknap petrknap force-pushed the snapshot-renamed-to-take-snapshot branch from f0cef91 to caf3ec5 Compare October 19, 2024 16:28
@petrknap petrknap marked this pull request as ready for review October 19, 2024 16:36
@petrknap petrknap merged commit c48d8d2 into main Oct 19, 2024
2 checks passed
@petrknap petrknap deleted the snapshot-renamed-to-take-snapshot branch October 19, 2024 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.

1 participant