-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Doesn't work with unsigned bigint numbers #12
Comments
Without even looking beyond the first few lines of code, this obviously doesn't work right for anything outside of the range It's a pretty sad and short-sighted design choice. The input starts as a string and results in a string. At no point should it have been turned into a number; that's just dumb. Your code just needs to be aware of how numbers work and manipulate strings according to basic number theory. At that point, your code's only limits are other things, like available memory. It doesn't matter if your code has been "validated against more than 2.78 million test assertions" if you don't document the obvious limits outside of which it fails to work. (And don't just document the errors and silently provide erroneous answers; throw an error to say you don't know how to count that high!) An error is always better than a wrong answer. It's just one of many design fails. For example, the function accepts input strings as hexadecimal, and even sometimes outputs answers in hexadecimal, e.g. Not to mention something like this unholy mess: (On the bright side, that one doesn't even compile, so you're going to know it's wrong. It's the ones that happen to be silently wrong that are the bigger problem.) Did you not know you were programming in JavaScript, which has implicit type conversions, no dedicated integer type and similar treacherous things? If you don't anticipate and catch errors yourself, it's going to silently give you garbage output. Even in range, this isn't very efficient, e.g.
|
Hey there, welcome to open source. This is where people collaborate to make code better for everyone. When I created this project it worked precisely for what I needed, and the use cases your mentioning would never be permitted in places where this was used. If you want to use it for something else, do a PR. If you don't, why are you so using so many words to say so little?
lol, "number theory"
Why would one document something obvious? Or were you trying to articulate something else. Please take your time.
Garbage in, garbage out.
Where is your regex range code so I can see how it's supposed to be done? Or really any examples you can show me of code you've written so we can all learn. Since you know how it should be done, it would be a shame if you kept that all to yourself. |
Not sure why that's funny. What I mean is the basic rules about how regular numbers work. For example you have
Sure. Obvious to a programmer looking at the code. Not obvious to a user/consumer of the module. The essence of a good module is having a well-defined interface and rules about what the module does. Packaging it in a module, among other things, is meant to hide irrelevant implementation details, so that it can be used in a variety of contexts as long as the well-defined interface is respected by the module and the module users.
That's not a virtue. Within a module, of course you'll use functions that don't check inputs as much, because you can make assumptions about what code is calling certain functions. But the exposed functions of a module should be checking inputs. To some extent you understood this, because you made a function to check inputs and threw TypeError. It's just that your checking wasn't complete.
You seem to be getting defensive or upset like I'd called your daughter ugly. It's just code, dude. I've written a lot of crappy code myself that did what I needed it to at the time. However, when you publish it as a module for others to use, then of course it will be exposed to scrutiny. And it should be, because your code is being used on websites that purport to provide working and correct regular expressions, and they don't. I notice that the original report didn't get the slightest acknowledgment in over four years, but when I commented, you replied in minutes -- not to acknowledge reported defects, but to get defensive. Interesting. What do you think the purpose of issues are if not to report, acknowledge and work on defects? Like you said,
And so that should be the goal. If your only concern is "it worked precisely for what I needed", then that isn't what making it better for everyone looks like. As to fixing this issue: Sure, if I get some extra time, I can code up what I had in mind to show you. But a simplistic fix for your existing code could at least:
Hope this helps. |
//outputs true, then false
The text was updated successfully, but these errors were encountered: