Computer Science (both courses): Improve space complexity content#30463
Computer Science (both courses): Improve space complexity content#30463JoshDevHub wants to merge 10 commits intoTheOdinProject:mainfrom
Conversation
mao-sz
left a comment
There was a problem hiding this comment.
I'll leave more of the content review to Austin but got a couple of nits and word flow suggestions below.
I've only attached the comments to the JS lesson diff but all of the non-code comments apply the same to the Ruby lesson of course.
Co-authored-by: mao-sz <122839503+mao-sz@users.noreply.github.com>
Co-authored-by: mao-sz <122839503+mao-sz@users.noreply.github.com>
Co-authored-by: mao-sz <122839503+mao-sz@users.noreply.github.com>
Co-authored-by: mao-sz <122839503+mao-sz@users.noreply.github.com>
Co-authored-by: mao-sz <122839503+mao-sz@users.noreply.github.com>
Co-authored-by: mao-sz <122839503+mao-sz@users.noreply.github.com>
|
@JoshDevHub in case you missed it in the review main comment, I have the same suggestions for the Ruby version of the lesson. I was just lazy with repeating the comments and suggestions on both files so I only stuck them on the JS one 😅 |
|
@mao-sz Oh I know. I've just been equally lazy about going to fix them myself manually 😅 |
|
Okay I finally put Mao's changes over in the Ruby lesson as well. Now this just needs signoff from someone on the DSA team 🚀 |
|
Apologies. I'll try to review in the next couple of days |
mao-sz
left a comment
There was a problem hiding this comment.
Sorry this took a while, Josh. Had a more proper read through of the new content and I think I'm happy with what it covers. Just some small tweaks/thoughts below.
| - [What are the main considerations we should consider before optimizing code?](#other-considerations) | ||
| - [What is a situation where auxiliary space analysis is useful?](#auxiliary-space-analysis) |
There was a problem hiding this comment.
I think you intended for this to be a replacement KC question instead of additional? Since the JS once has it replace the optimizing question, and the Other considerations heading has gone too
| - [What are the main considerations we should consider before optimizing code?](#other-considerations) | |
| - [What is a situation where auxiliary space analysis is useful?](#auxiliary-space-analysis) | |
| - [What is a situation where auxiliary space analysis is useful?](#auxiliary-space-analysis) |
|
|
||
| ```javascript | ||
| function squareNumsInPlace(arr) { | ||
| for (let i = 0; i < arr; i++) { |
There was a problem hiding this comment.
| for (let i = 0; i < arr; i++) { | |
| for (let i = 0; i < arr.length; i++) { |
| function squareNumsNewArr(arr) { | ||
| const squaredNums = []; | ||
| arr.forEach((number) => { | ||
| squaredNums.push(number * number); | ||
| }); | ||
|
|
||
| return squaredNums; | ||
| } |
There was a problem hiding this comment.
Is the use of an empty array and forEach+push to be extra extra explicit about the use of an additional array? Or would using .map suffice? Since at this point, .map should both be more idiomatic and probably sufficiently clear that an extra array is involved if the following paragraph is tweaked accordingly.
Not sure if it's as idiomatic in the Ruby version.
Ultimately, not married to this if the idea is to be extra extra explicit about an additional array being involved.
Because
There are a few things that went into this.
The lesson previously included this blog post in the assignment: https://dev.to/mwong068/big-o-space-complexity-lcm
A lot of the explanation of auxiliary space was off-loaded to this post, but it has a couple of inaccuracies:
This blog post has caused repeatedly caused confusion for learners (see the linked issue for one example).
There was a previous section in the lesson that included references to the different approaches one can take when counting space. I decided to merge this into the new auxiliary space section. It covered similar ground, but I also thought the way it was previously worded was a bit unfair to auxiliary space analysis. At least in my experience, it's fairly common to use it, especially when comparing two algorithms that take the same input. This can even be seen when the lesson links to the BigO cheatsheet and multiple sorting algorithms are described as taking up constant space, meaning they're definitely using an auxiliary space lens.
This PR
(All changes are made in both the Ruby and JS versions of the lesson)
#### Other considerationssection and merge it with the new auxiliary space section.Issue
Closes #29254
Additional Information
Pull Request Requirements
location of change: brief description of changeformat, e.g.Intro to HTML and CSS lesson: Fix link textBecausesection summarizes the reason for this PRThis PRsection has a bullet point list describing the changes in this PRIssuesection