Setting the internal counter value#7
Conversation
Added possibility to set: * initial value for the counter * counter value limits (min and max)
Added description of additional options with respect to the original
Added description of new features in the for with respect to the original
nstansby
left a comment
nstansby
left a comment
There was a problem hiding this comment.
Great PR, happy to merge it but there are a few things I'd like to clean up first. Thanks for contributing.
| if self.value - 1 < self.minValue: | ||
| self.value = self.minValue | ||
| else: | ||
| self.value = self.value - 1 |
There was a problem hiding this comment.
Any reason not to just do self.value = max(self.minValue, self.value - 1)? It's more concise, and (to me at least) it makes the intent of the code easier to undertand.
| if self.value + 1 > self.maxValue: | ||
| self.value = self.maxValue | ||
| else: | ||
| self.value = self.value + 1 |
There was a problem hiding this comment.
Similar to above, but use min()
| # Description | ||
| This simple code is a fork of great inital work made by nstansby. I've added tew tweaks so I can use the code for my little project | ||
| * possibility to set the min and max limits for the counter | ||
| * possibility to initiate the counter with an arbitrary value | ||
| * capability of setting the value of the counter at any time | ||
|
|
There was a problem hiding this comment.
Can you change these so that they'll make more sense when merged... people seeing it will not be looking at a fork, but at the real repo.
| ```python | ||
| from encoder import Encoder | ||
| e1 = Encoder(26, 19) | ||
| e1 = Encoder(26, 19, 0, 100, 20) |
There was a problem hiding this comment.
I'd prefer to use the explicit notation e1 = Encoder(26, 19, minValue=0, maxValue=100, initialValue=20). Once it becomes a long string of unnamed integers, it's pretty much impossible for someone reading the code to understand without going and looking at the function declaration. Yeah, I know you could argue that's true for the first two arguments, and we're all using IDEs and can trivially jump to the declaration, but there's just something about e1 = Encoder(26, 19, 0, 100, 20) that offends my sensibilities!
2 participants
I've added few lines that might be useful for controlling the internal counter value. I think it might be a great addition to the work already done.