{"action":"created","issue":{"url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/83","repository_url":"https://api.github.com/repos/AtheMathmo/rulinalg","labels_url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/83/labels{/name}","comments_url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/83/comments","events_url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/83/events","html_url":"https://github.com/AtheMathmo/rulinalg/pull/83","id":186557885,"number":83,"title":"Sparse Matrix implementation","user":{"login":"c410-f3r","id":17877264,"avatar_url":"https://avatars.githubusercontent.com/u/17877264?v=3","gravatar_id":"","url":"https://api.github.com/users/c410-f3r","html_url":"https://github.com/c410-f3r","followers_url":"https://api.github.com/users/c410-f3r/followers","following_url":"https://api.github.com/users/c410-f3r/following{/other_user}","gists_url":"https://api.github.com/users/c410-f3r/gists{/gist_id}","starred_url":"https://api.github.com/users/c410-f3r/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/c410-f3r/subscriptions","organizations_url":"https://api.github.com/users/c410-f3r/orgs","repos_url":"https://api.github.com/users/c410-f3r/repos","events_url":"https://api.github.com/users/c410-f3r/events{/privacy}","received_events_url":"https://api.github.com/users/c410-f3r/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2016-11-01T15:07:00Z","updated_at":"2016-11-06T09:28:47Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/AtheMathmo/rulinalg/pulls/83","html_url":"https://github.com/AtheMathmo/rulinalg/pull/83","diff_url":"https://github.com/AtheMathmo/rulinalg/pull/83.diff","patch_url":"https://github.com/AtheMathmo/rulinalg/pull/83.patch"},"body":"Same PR, but with no arithmetic operations."},"comment":{"url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/comments/258667045","html_url":"https://github.com/AtheMathmo/rulinalg/pull/83#issuecomment-258667045","issue_url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/83","id":258667045,"user":{"login":"Andlon","id":578309,"avatar_url":"https://avatars.githubusercontent.com/u/578309?v=3","gravatar_id":"","url":"https://api.github.com/users/Andlon","html_url":"https://github.com/Andlon","followers_url":"https://api.github.com/users/Andlon/followers","following_url":"https://api.github.com/users/Andlon/following{/other_user}","gists_url":"https://api.github.com/users/Andlon/gists{/gist_id}","starred_url":"https://api.github.com/users/Andlon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Andlon/subscriptions","organizations_url":"https://api.github.com/users/Andlon/orgs","repos_url":"https://api.github.com/users/Andlon/repos","events_url":"https://api.github.com/users/Andlon/events{/privacy}","received_events_url":"https://api.github.com/users/Andlon/received_events","type":"User","site_admin":false},"created_at":"2016-11-06T09:28:47Z","updated_at":"2016-11-06T09:28:47Z","body":"@c410-f3r: thanks for the overview! Definitely makes things a little easier to follow.\r\n\r\nAbout the code comments part: I definitely agree with the idea, and the two books you quote are classics in the field. Generally, if you can, your code should be self-documenting. That is, the name of your functions and variables should explain what it is you're doing. Now, what it often cannot do, is explain _why_ you are doing something, or to justify a particular course of action. Moreover, you might find applying those principles difficult when writing low-level data structures (which includes the compressed matrix formats) and when working with mathematics, in which case things may be defined only mathematical concepts, and consequently coming up with good names might be excruciatingly difficult.\r\n\r\nRegarding the _why_, consider the following silly example:\r\n\r\n```rust\r\n// First saddle the horse\r\n{ ... }\r\n\r\n// Put shoes on horse\r\n{ ... }\r\n\r\n// Give horse ice cream\r\n{ ... }\r\n\r\n// Ride horse\r\n{ ... }\r\n```\r\n\r\nNow, using the principles you mentioned, we can greatly improve this without comments at all:\r\n\r\n```rust\r\nsaddle_horse(&mut horse);\r\nput_shoes_on_horse(&mut horse);\r\nfeed_horse_ice_cream(&mut horse);\r\nperson.ride(&horse);\r\n```\r\n\r\nGreat! Our code is now self-documenting. But why the hell are we giving the horse ice cream before riding it? This calls for a comment!\r\n\r\n```rust\r\nsaddle_horse(&mut horse);\r\nput_shoes_on_horse(&mut horse);\r\n\r\n// Our horse is unfortunately allergic to carrots, and is a very picky eater.\r\n// From experience, the only thing that will make the horse cooperative is ice cream\r\nfeed_horse_ice_cream(&mut horse);\r\nperson.ride(&horse);\r\n```\r\n\r\nAh, so that's why.\r\n\r\nNow, I'm sure the code in question can be organized better into smaller functions. Just as an example, you might want to do something like `determine_sparsity_pattern()` in the `from_triplets` function, which first determines the indices and row pointers before putting the data in. I don't know if this particular example is appropriate, but this kind of refactoring might help make the code easier to read and understand!"}}
{"actor":{"display_login":"Andlon"}}