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
Snippet restructuring #233
Comments
For starters we need to improve descriptions and explanations to make sure all snippets are well-explained and devs understand their usage. However, this contradicts the original idea of 30 seconds of code, that being that the snippets and explanations being brief (although at this point many of the original ideas about the project have gone out of the window). As far as usage goes, I saw your comment under #228, too, and I agree we need to make the usage separate, but I will have to retroactively update some snippets, so that we can use the web builder to do everything properly. Also, this will have to wait for a few dates, so that my CSS framework has a working collapse in the latest version in order to allow us to show/hide examples. As far as separating the snippets into the suggested sections (descriiption, usage, explanation) that is definitely possible by updating the wb script, too, meaning we don't have to change the existing snippets, but we just have to be smart about the way we parse them. We could also do the same for the markdown version, but I have a bad feeling about this. Any other opinions? |
I like the idea of having a more detailed explanation of the snippet, also having them in the website as oposed to the project README is probably the best solution for this. This boils down to the complexity inherent to each snippet, some of them are really easy to grasp, but then again, this also depends on the reader's proficiency with some concepts, also the majority of the snippets are highly optmized, which is a good thing, but it usually takes the readability down. It's a trade-off, traditionally highly optimized code tends to need a more robust documentation. I guess we need to think where do we stand conceptually to move forward with this. |
I believe that such ordering of the presentation needs more clarity and I think you're on to something about that since it looks like it follows close to how I have seen things in the past with other libraries and repos, ie:
Another advantage is that with usage specifically separated from snippet and not in comments so you can easily find and copy the snippet without hitting any comments, and if you wanted to grab some example use cases too then it would be easy. A problem arises with in depth explanations of whats happening since it would be contributors going out and trying to re-explain the wheel. I think we should not do/require in depth explanations. If someone is curious what an explanation means they should probably either look in a FAQ, or join the gitter and ask about it for a more in depth explanation of what they don't understand. Getting to a clear explanation is much harder, regardless of how long it is. However I do believe function signatures might be outside the scope of the repo since it's mainly a place to learn some cool tricks in a way you might not have seen. Now that's not to say some explanations can't be improved. They obviously can. Example Function SnippetsGreater Than
const gt = (a,b) => a < b gt(6,3) // false
gt(3,4) // true
const greaterThan6 = gt.bind(null, 6)
greaterThan6(3) // false Call Helper
const call = (key,fn) => context => context[key](fn) Promise.resolve(['String1', 'sTring2']).then(call('map', s => s.toUpperCase()) Array Max value
const arrayMax = a => Math.max(...a) arrayMax([1,3,2,5,9,100]) // 100 |
As a side note, I do agree |
This is exactly what I thought about after I made this issue. Basically, we need to define a level of assumed proficiency. I believe that can be the "meta" of snippets, such as using ES2015 syntax (arrow functions) - they should understand that, so that doesn't need to be explained (i.e. implicit return), otherwise you'd need to explain every snippet. But the content of the snippet (the guts of the function) should be explained to a sufficient degree imo. |
I feel like that's a valid complaint. If the contents of a snippet can't be explained sufficiently then it needs an update.
versus
For that matter we should make a snippet for SpreadGiven a const butter = (f,a) => f(...a) const spreadC = f => a => f(...a) Math.max.apply(null, [1,2,3]) // 3
butter(Math.max, [1,2,3]) // 3
var max = butter.bind(null, Math.max)
max([1,2,3]) // 3
spreadC(Math.max)([1,2,3]) // 3 |
Great points overall. Defining the assumed level of user proficiency is pretty important, as it could save us a lot of headaches in the long run. My original vision was to provide snippets for people that know Javascript reasonably well, but are too lazy or don't want to rewrite the same 10 methods in every project. However, the project has definitely grown out of that, so I suspect that that definition is entirely out of date by now. What I believe is a decent assumption:
Of course the above is my personal opinion, so feel free to speak your mind, so we can reach a mutual agreement on what we consider an average visitor to the website or README file of the project. |
The good news is we've already outlined that most snippets are made out of the following
The only question then becomes the markup for the overall snippet structure, what way are we leaning? Should we have some sort of vote of a few prescribed structures by reaction? Example Voting PostHow to VoteUsing a reaction to this post put in your vote for which structure we should adopt 👍 : proposed by @...
👎 : proposed by @...
😆 : proposed by @...
... |
Warning this will mean once this is adopted we will need to go through and update each snippet, and allow others to contribute updates as well |
My suggestion after thinking this through is the following:
isEvenReturns Checks whether a number is odd or even using the modulo ( const isEven = num => num % 2 === 0;
/*
isEven(3)
// false
isEven(6)
// true
*/ Explanation of the above sample: We should use a multiline comment to showcase example use-cases. We should add expected output as comments, so that people can copy paste the whole piece from the website without having to deal with our annoying Opinions appreciated. |
@Chalarangelo good ideas, here's my opinion. Given this:
I think the README snippets shouldn't bother with the watered-down explanation - the in-depth explanation can be reserved for the website? And imo still split up the usage because it maintains the syntax highlighting. Structure for the README should be standardDeviationReturns the standard deviation of an array of numbers. const standardDeviation = (arr, usePopulation = false) => {
const mean = arr.reduce((acc, val) => acc + val, 0) / arr.length;
return Math.sqrt(
arr.reduce((acc, val) => acc.concat(Math.pow(val - mean, 2)), [])
.reduce((acc, val) => acc + val, 0) / (arr.length - (usePopulation ? 0 : 1))
);
}; Usage: /** Sample standard deviation **/
standardDeviation([10,2,38,23,38,23,21]); // 13.284434142114991
/** Population standard deviation **/
standardDeviation([10,2,38,23,38,23,21], true); // 12.29899614287479 Optional titles for the website are put inside A link to the website with the full explanation is given beneath. |
@atomiks I disagree with that. Our landing page is the repo's README. We want people to check the list of snippets and possibly find what they need and also understand it. The main goal of the project is to teach people how to code simple methods they need everyday, so stripping the how from the README (which is what 90% of the visitors see) seems like a really bad idea. |
The problem is the current descriptions/explanations are superficial and barely explain the code. They just say what to use, but don't explain why they're being used. That's what my OP was about. They would require a lot more detail to actually teach people. |
This boils down to the assumed level of proficiency of visitors, as discussed before. For an intermediate developer, mentioning the methods used is more than enough to be able to read through 5-10 lines of code and get a decent grasp of what the code does (this definitely does not apply to all snippets at this time, though). If we assume all the visitors are beginners in JS and ES6, we need to write really long and detailed explanations, which would beat the purpose of the project, as it would be more like 2.5 minutes of code at that point. Assuming that we have to deal with intermediate developers (which is actually the majority of web developers in my understanding based on the articles I read on Medium and dev.to and questions I see asked on SO) lets us simplifiy descriptions to an extent, so that people do not have to read 20 lines of detailed explanation. From the feedback I have gathered, people prefer byte-sized pieces of information, much like our snippets. Part of what makes this project so popular so far seems to be that. |
Yeah, good point. But I think the people who need an explanation the most are the novice to near-intermediate devs. So the short/shallow explanations are kind of pointless because to intermediate-advanced devs, the code generally explains itself. At the very least, we can strike a balance between what we have currently and a thorough explanation. So for the OP I made, a balance might look like this:
|
@atomiks that seems like a decent enough balance, as it is not too long and helps people understand a little bit more in depth, allowing them to even skip the code snippet and just read the description. That seems like a decent compromise that will help us acommodate the needs of more people. |
Do you also agree with splitting the usage? I dislike the usage-in-code block thing that we have currently as it:
|
@atomiks I would like to split the usage, although we need to update both build scripts, which will be something I'm not very willing to do for the next week, as I will be on vacation. If anyone can undertake that project, I would be glad to get these changes underway. |
^ Yeah the "usage" is probably redundant except for the first couple of snippets to a new reader. I'm sure it's obvious, and the quick/easy copy-pasting is a bonus. I'm not sure with that description though, as it's not really that clear and would be a bit too confusing for new devs |
We assume they know what spread operators and implicit returns are don't we? |
Well the "meta" of the snippets (arrow functions), as mentioned before, is assumed, but not the content of it. So you'd need to give a bit more detail of the inside of the function. Another example: capitalizeEveryWordCapitalizes the first letter of every word in a string.
Use const capitalizeEveryWord = str => str.replace(/\b[a-z]/g, char => char.toUpperCase()); capitalizeEveryWord('hello world!'); // 'Hello World!' |
@atomiks Although slightly long as a description, it's a good example of an explanation done right. I wouldn't disagree with retroactively updating snippets to have better and longer explanations. However, we must first decide how to deal with our examples problem (which I think pretty much everyone agrees should be moved to a second code block below the actual code) to start retrofitting snippets. |
Hey All 👋 Although I'm joining the discussion kinda late. I def loving the second code block idea. |
Alright, se we are going to update CONTRIBUTING.md guidelines and retroactively alter all snippets to use a second block for examples? Before we do that, we need to upgrade our linter to make sure it doesn't break due to this change (and it will), update our snippet_template and also deal with the way our copying to clipboard is implemented to only add the button to Oh, also I'm on vacation, so expect me to join in on the updates and fixes after Dec. 27. |
I am also late to the discussion, but here is my opinion: But first of all, we need to update our linter and build scripts. |
Update@fejes713 has already started restructuring existing snippets in a new branch, so we can update and test all the scripts before applying the restructure and finalizing our guidelines. The new format will be as follows (based on this discussion):
I will try to figure out ways to update our scripting and make sure everything works as intended before we merge the branch and close the issue. Afterwards, we can work on an improved styleguide (as there are some issues) and add testing to have everything set up. Thanks to all of you who contributed to the discussion either here or on the gitter channel. |
Hey @Chalarangelo what do you think about having a toggle visibility for the code block with the examples? |
@Rob-Rychs that is not part of the restructure strictly, but I think we will do that. I will have to work on that personally, because the CSS is kinda tricky to play around with and undocumented (partially), but it will be online eventually, so no worries! 😉 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks. |
The current snippets are designed to be short/brief, but at the cost of sacrificing legibility. For example, the first snippet is:
I noticed when copy/pasting, the comments kinda got in the way. Ideally, you'd want to separate the definition and the usage.
Additionally, the description is a bit lackluster. Why is the spread operator and Math.max() being used? If you know how it works, then it's obvious, but to new developers reading the code they probably don't know what's happening.
I suggest you keep the
what
of the function at the top, then add a comprehensivehow
beneath.arrayMax
Returns the maximum value in an array.
Usage
Explanation
Math.max()
returns the largest value given to it, accepting each value as a separate argument. For example,Math.max(1, 5, 3, 0)
returns5
. However, it doesn't take an array of values - they must be supplied as separate arguments.To find the largest value of an array, we can use the spread
...
operator.Math.max(...[1, 5, 3, 0])
is the same asMath.max(1, 5, 3, 0)
. The spread operator "spreads" the values of the array into the function as separate arguments, which returns the largest value, thus returning the largest value of an array.The clear downside of this is decreased information density. Ideally you'd have the usage and explanation collapsed, only to be expanded if necessary, but that's not possible(?) with MD. But it seems like a perfect opportunity for the website. But then you have 2 different versions 😓
Thoughts/ideas?
The text was updated successfully, but these errors were encountered: