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

Better to add timeout/interval to the Instruction class instead of ConditionWatcher? #4

Open
hkmushtaq opened this issue Jul 24, 2017 · 3 comments

Comments

@hkmushtaq
Copy link

I think it makes more sense to make the interval/timeout be instruction specific then be on on the main ConditionWatcher class.

We can keep defaults but I feel like its safer for the defaults to reside in Instruction

I'm also willing to make the PR for it.
Thoughts?

@polok
Copy link

polok commented Jul 26, 2017

You have 👍 from me :)

@FisherKK
Copy link
Collaborator

FisherKK commented Jul 26, 2017

Hey @hkmushtaq

Sounds not bad I think.
Usually I created wrapping code around my ConditionWatcher's waitForCondition method. It resulted in even 15+ variations of something like this:

    public void waitForCondition(String key, int waitTimeInMs, int interval, Bundle data) throws Exception {
        Instruction instruction = InstructionStore.getInstructionForKey(key);
        instruction.setData(data);
        ConditionWatcher.setTimeoutLimit(waitTimeInMs);
        ConditionWatcher.setWatchInterval(interval);
        ConditionWatcher.waitForCondition(instruction);
        ConditionWatcher.setWatchInterval(ConditionWatcher.DEFAULT_INTERVAL);
    }

Your idea could reduce it so I think it's a good direction. Go ahead with PR, I will be happy to check and merge it :)

@hkmushtaq
Copy link
Author

Awesome, I'll get to work on refactoring it!

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

No branches or pull requests

3 participants