Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants