-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
refactoring _.some #2999
Comments
I think multiple things. Firstly, I think you made a good observation. You can express For what it's worth, you could also do the opposite, i.e., express Secondly, code changes should not be taken lightly, especially not in an established library that is being used in millions of projects. Changes in implementation can cause all kinds of subtle changes in behavior. We have good tests that should catch most of those, but something could escape our attention in theory, and there is the off chance that it negatively affects multiple dependent projects. Such a risk can easily be justified if the change has some clear benefit for everyone to compensate, but this still means we should not change the implementation of a function simply because we can. Thirdly, therefore, I invite you to think about the rationale for the change that you proposed. Would you risk subtly breaking changes just to make the code a little bit more elegant? If yes, why? If not, can you think of another benefit that does justify the risk? I suggest that you reflect on this before you continue reading my fourth thought. I will be curious to read your answer. Fourthly, I have actually been working on a large pull request that includes a similar change for over four years. You can read how I rewrote I consider this a ticket that will be closed when #2908 is merged or when your question is answered, whichever happens first. I'm guessing the latter. |
Haha wow thank you so much for all of this! I have to admit that a big part of me was like ‘obviously if it’s the way it is there’s a good reason for it, you idiot, those guys are pros and you know nothing, so shut up and keep learning!’, but a smaller (but louder) part of me just wanted to feel smarter than everyone else, only to be very predictably proven wrong immediately afterwards. The worst part is, I do know about the risks of change - just a few days ago I read about the case of typeof null === object and why a proposed change to === null was rejected - but the ego is always eager to dismiss the wise ways and a kind reminder like yours is very welcome. In this case though, both implementations seemed to me like they were doing the exact same thing and there would be no negative consequence. But I mean, I looked at them for two minutes, while it’s been taking YOU over four years to assess the impact of the changes in your rewriting, so… As for the benefits, the rationale, to me, wasn’t about elegance - which serves no useful purpose and, I know first hand, can hurt a program’s readability - but rather, well… better readability. Although I now realize that 1. something that’s more readable to me isn’t necessarily for someone else, and 2. if one has figured out _.every, then the work of understanding _.some is pretty much done already anyway, and any new implementation, even a more readable one, would require unnecessary work. So the benefits here are... not much. Not to mention, as you point out, the risk of breaking something in millions of projects. I think my message was simply that of a frustrated newbie struggling to understand much of the code and going ‘come on, let’s make this easier!’. I deeply appreciate your time and patience and pedagogy in the face of my cocky approach (like I was going to teach you logic haha), and you giving me some more stuff to learn about, like this _.find function I hadn't heard of yet. I’ll definitely have a look at your rewriting of _.some and _.every and this large pull request too. Thanks again! |
You're welcome. I did intend to teach you a little, but please don't take that as a call to humble yourself. Your observation was still correct, and this is something to appreciate. While I wanted to help widen your perspective a bit, I only felt this was worthwhile because I thought you already had a good perspective to start with. That is not only because you made the logical observation, but also because you showed the willingness to contribute and the courage to make a suggestion. I think you were right not to listen to your self-deprecating part. There is a lot of room for nuance here as well. Elegance and simplification are far from useless; they do contribute positively to code quality in general. It's just that there are many tradeoffs to keep in mind. I like to think of "readability" as having two components: recognizability, which is highly subjective, and "unclutteredness" (for lack of a better word), which is much less subjective and even to some extent measurable. Functional style tends to reduce clutter, which in my view makes it inherently a positive force for readability, even when some readers don't recognize Is your question answered? In that case I will close the ticket, although we can still continue the discussion if you like. |
Don't worry, I'm making the call to humble myself... myself. :) Come to think of it, I can see how elegance (vs. showing off) can actually make the code enticing and fun to read and work with. That's at least one benefit I can think of. Anyway, yes my question is answered (along with a bunch of others I didn't even have!), you can close the ticket, thank you very much.(Or should I just do it myself? I'll do it.) I should really focus on the rest of my pre-bootcamp course for now, but I'll come back and have a look at your responses again once in a while, there's a lot for me to learn or re-learn here. |
If I may add some uninvited perspective, I agree with Julian that there isn't a need for humbling yourself too much. On contribution:The contents of the conversation and insights discussed in this issue are exactly the reason why I started following the development of underscore, backbone and backbone marionette years ago and still watch the repositories to this day even though I don't use them in the projects that currently paying my bills. There is much to be learned by following the thought processes behind the development of tools that gets used by many people, particularly so when one of the goals is to try and not be too opinionated about how the tool should be used. Truth is both the type of thinking behind the original question and the way of looking at the responsibility towards the people who use the library are highly valuable and both contribute to the quality of the library. I think this is why Julian expressed curiosity about your thoughts. The potential for him to learn something from you was as real as the potential for you to learn something from him. Like Julian said, it takes a little courage to enter the conversation and suggest improvements or discuss pros and cons of other people's suggestions on a serious project that has been going for a while, but I believe it's extremely worthwhile. It's certainly improved my skills and I'm convinced it's one of the best ways to grow and gain experience. I would encourage you to keep interacting. On clean code:On the topic of readability and cleanliness of code, I think one aspect I personally try to keep in mind is to try and minimize cognitive dissonance. Taking composition as an example, wat we're really doing there is to compress algorithms or processes into single tokens and combining those. To understand the composition, we need to unpack those compressed representations enough to understand what we're reading. The beauty and power of composition lies in the fact that our unpacked mental model of the individual tokens doesn't necessarily need to contain all the intricate implementation details. Often times the higher level concepts are enough. The question of cleanliness of code often involves estimating the amount of mental effort involved in unpacking compressed representations of underlying complexity. If our audience can maintain an intuitive understanding of the abstractions they are confronted with, the code feels clean. Whenever we break that intuition we introduce cognitive dissonance and the sensation of cleanliness is reduced. The process is subjective because maintaining intuitive understanding depends a lot on our audience. To give an admittedly slightly extreme example, a junior Rust developer will probably have no fear of monads, but a junior WordPress theme developer will probably have absolutely no idea what those are. |
I feel that everything each of you has said will be very valuable to me in order to become a good developer. |
Hi
As part of an assignment before a JS bootcamp, I had to re-write some of the Underscore functions - sometimes in a simplified version. I just had a look at the original implementation of _.some, and I thought I might suggest simply (your adaptation of) this instead:
It's based on
What do you think?
The text was updated successfully, but these errors were encountered: