Skip to content

Blog lesson 7 instructions aren't clear enough #68

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

Closed
terichadbourne opened this issue Oct 22, 2018 · 9 comments
Closed

Blog lesson 7 instructions aren't clear enough #68

terichadbourne opened this issue Oct 22, 2018 · 9 comments
Assignees

Comments

@terichadbourne
Copy link
Member

A few issues I ran into when trying in vain to solve blog lesson 7 (https://proto.school/#/blog/07). I'll need help better understanding some context here so I can then phrase instructions and tips more clearly to make this one easier.

  1. When do you need your CID to be an instance and when don't you? I overused the new CID(...) that was suggested here in my first attempt, assuming it was necessary in places it wasn't. Are the things returned by ipfs.dag.put() already CID instances? Is doing the new CID thing the opposite of doing the someCid.toBaseEncodedString() thing? You have to do it to decode it?

  2. It's unclear which things in this code block are meant to be written as is and which are meant to be replaced by some other value: let cid = new CID((await ipfs.dag.get(someCid)).value.linkName['/'])
    I could tell, because of the "some," that someCid was meant to be replaced, but I didn't make that connection for linkName. I now believe you're supposed to replace this with the name you gave the object that contains an object in which the field name is '/' and the value is the someCid.toBaseEncodedString, which in this case would be prev. Maybe we should use fieldName or someLinkName or linkFieldName here? Or use coloring or wrap in <> symbols to highlight both of the segments you're supposed to replace (in which case we should do that throughout all lessons)? (Note that there's something related about naming of links at https://proto.school/#/blog/03 )

  3. I think we should give a hint about using a while loop. IMHO, less experienced developers have more experience looping through arrays where they know how to judge the number of entries and less with figuring out how to make their loop stop itself by other means. Maybe be so explicit as to say something like:
    Hint: How do you know when you're out of links? Try using a `while` loop and checking for the presence of a `prev` field in the current object. If it exists, you're not done yet, and you'll need to reset a variable and run the loop again.
    I'd like people spending their time thinking about how to access the right part of the CID object via IPFS commands, not thinking about the logic of the code.

  4. I only solved this by looking at the solution provided in the underlying code, and then I was a little confused by the difference between what's done here, where you take the value from the top level of the object and then dig into value.prev as opposed to what was taught in basics lesson 3 (https://proto.school/#/basics/03) where the implied structure for fetching this would have been ipfs.dag.get(cid, '/prev/'), in which the returnedvalue would be a string, not an object containing the prev field. Am I right to assume those are alternatives that should both work? If so, should we consider pointing out the two alternatives in that earlier lesson, or calling it out here with a link back to that lesson?

  5. I was confused about why I didn't have to change return traversePosts near the end of the starter code to return traversePosts(<theLatestCid>). In function itself can't do anything without a cid passed in, and in the other exercises we've had to return something specific like that using variable names we established in the code. It seems like this exercise is built to return just the function itself to be tested. The instructions state that the traversePosts function should return an array of all the blog CIDs, but I'd recommend adding something like "There's no need to change the return value on line X to take an argument. Just ensure that, if an argument were passed into the function, the right array would be returned from your traversePosts function.

@olizilla @mikeal @vmx If any of you have suggestions before I try to clarify these instructions, they'd be appreciated. Thanks!

@terichadbourne terichadbourne self-assigned this Oct 22, 2018
@vmx
Copy link
Collaborator

vmx commented Oct 24, 2018

  1. When do you need your CID to be an instance and when don't you? I overused the new CID(...) that was suggested here in my first attempt, assuming it was necessary in places it wasn't. Are the things returned by ipfs.dag.put() already CID instances? Is doing the new CID thing the opposite of doing the someCid.toBaseEncodedString() thing? You have to do it to decode it?

Currently ipfs.dag.put() doesn't return CID instances, but it will, once js-ipfs ships with a more recent js-ipld.

  1. It's unclear which things in this code block are meant to be written as is and which are meant to be replaced by some other value: let cid = new CID((await ipfs.dag.get(someCid)).value.linkName['/'])
    I could tell, because of the "some," that someCid was meant to be replaced, but I didn't make that connection for linkName. I now believe you're supposed to replace this with the name you gave the object that contains an object in which the field name is '/' and the value is the someCid.toBaseEncodedString, which in this case would be prev. Maybe we should use fieldName or someLinkName or linkFieldName here? Or use coloring or wrap in <> symbols to highlight both of the segments you're supposed to replace (in which case we should do that throughout all lessons)? (Note that there's something related about naming of links at https://proto.school/#/blog/03 )

I agree that it is unclear. I like the idea of brackets in addition to more text mentioning them explicitly that they should be replaced. BTW with the newer version of js-ipld this will get a bit simpler:

let cid = (await ipfs.dag.get(someCid)).value.linkName
  1. I think we should give a hint about using a while loop. IMHO, less experienced developers have more experience looping through arrays where they know how to judge the number of entries and less with figuring out how to make their loop stop itself by other means. Maybe be so explicit as to say something like:
    Hint: How do you know when you're out of links? Try using a `while` loop and checking for the presence of a `prev` field in the current object. If it exists, you're not done yet, and you'll need to reset a variable and run the loop again.
    I'd like people spending their time thinking about how to access the right part of the CID object via IPFS commands, not thinking about the logic of the code.

Good idea.

  1. I only solved this by looking at the solution provided in the underlying code, and then I was a little confused by the difference between what's done here, where you take the value from the top level of the object and then dig into value.prev as opposed to what was taught in basics lesson 3 (https://proto.school/#/basics/03) where the implied structure for fetching this would have been ipfs.dag.get(cid, '/prev/'), in which the returnedvalue would be a string, not an object containing the prev field. Am I right to assume those are alternatives that should both work? If so, should we consider pointing out the two alternatives in that earlier lesson, or calling it out here with a link back to that lesson?

With the newer version of js-ipld, value.prev will be a CID object. A ipfs.dag.get(cid, '/prev') resolves the link, so you will get the object which is behind the CID of value.prev.

  1. I was confused about why I didn't have to change return traversePosts near the end of the starter code to return traversePosts(<theLatestCid>). In function itself can't do anything without a cid passed in, and in the other exercises we've had to return something specific like that using variable names we established in the code. It seems like this exercise is built to return just the function itself to be tested. The instructions state that the traversePosts function should return an array of all the blog CIDs, but I'd recommend adding something like "There's no need to change the return value on line X to take an argument. Just ensure that, if an argument were passed into the function, the right array would be returned from your traversePosts function.

Sounds good. I tried to keep the needed changes to a minimum, hence you only need to change the function itself.

@terichadbourne
Copy link
Member Author

Thanks for the feedback, @vmx!

@mikeal
Copy link
Member

mikeal commented Nov 2, 2018

Currently ipfs.dag.put() doesn't return CID instances, but it will, once js-ipfs ships with a more recent js-ipld.

Yes it does :) Well, to be more specific, it returns a promise that resolves to a CID instance. You can test this using console.log() in one of the lessons :)

console.log(await ipfs.dag.put({hello: 'world'}))

@terichadbourne
Copy link
Member Author

Just spoke with @mikeal and we decided to wait on clarifying the instructions for this lesson until the API gets updated, thereby allowing us to have simpler instructions. :)

@mikeal
Copy link
Member

mikeal commented Nov 2, 2018

@vmx I've seen a ton of the deps getting updated to change this API, about when do we think this will land in js-ipfs?

@vmx
Copy link
Collaborator

vmx commented Nov 2, 2018

@mikeal The CBOR stuff is all merged, it now takes and returns CIDs. That's part of the 0.33 release. which was released yesterday.

@mikeal
Copy link
Member

mikeal commented Nov 5, 2018

Logged a new issue about the migration. Closing this issues.

@mikeal mikeal closed this as completed Nov 5, 2018
@terichadbourne
Copy link
Member Author

Reopening as lesson 7 is still not easy enough, despite the changes created by the new API (to be merged shortly in #79). I added a hint about the while loop there and @mikeal changed the code to use the new simpler API so some of the complexities could be removed from the instructions.

My success rate solving this lesson with the new API is about 1 out of 10 and is only that high because I cheated.

Unsolved problems include:

  • If you did the Basics lesson before this one, you'll remember having used ipfs.dag.get(cid, 'prev') and there's no explanation here of why we're introducing (await ipfs.dag.get(cid)).value.prev instead. The only solution I've ever gotten to work is the one I copied from the solution code, so I don't know if both options are possible or not. Can someone help me understand why the value needs to be used here, or provide me with a code sample using the other version that doesn't stall out on the new site?

  • It's too easy to write code that stalls out with too many awaits in it (I added a hint about this in the lesson). Even just the timing of when you adjust your previous value within the loop can be the difference in whether this works or not.

@terichadbourne
Copy link
Member Author

@vmx @mikeal - @fsdiogo and I just merged a bunch of updates to try to improve the clarity of this most dreaded lesson. 😂

I still presume it's going to be possible for people to get caught in an endless await cycle, but I seem to have forgotten how to get myself into that particular breed of trouble.

I'm closing this issue, but please feel free to open a new issue if you see room for improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants