Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Show solutions from top users on problem solutions page #235

Merged
merged 2 commits into from Apr 11, 2013

Conversation

Projects
None yet
2 participants
Contributor

hans commented Mar 20, 2013

I've introduced many people to Clojure by way of 4Clojure, and several have been confused on why they need to follow users to see other solutions. This patch adds code to display a few of the solutions from the top users for a given problem. (Somewhat related to #152.)

Screen Shot 2013-03-20 at 10 39 48 AM

Alternatively (/ additionally), we could show solutions from random users. Please let me know your thoughts!

Thanks,
Hans Engel

Owner

amalloy commented Mar 26, 2013

I love the general idea of this pull request, but I'm a little wary of some details. In particular, you're grabbing the top users, and then for each one going to the database and asking for their solution; it would be much better to use mongo's $in query to ask the question all in one go. There's also a limit argument you can pass to make sure you only get five results back, so that you won't need the take/filter approach.

The history of these commits is also quite messy; I'd appreciate if you could rebase all the nonsense commits like "typos" out and present a cleaner patch series. If you're not familiar enough with git to do that, it's fine and I can do it, but if possible I'd rather you do.

Anyway, thanks a lot for adding this feature: I think it will help the initial experience of new users quite a bit.

Contributor

hans commented Mar 28, 2013

Thanks for the feedback! My apologies for these egregious errors—I'm only just getting started with Clojure web development and MongoDB.

In particular, you're grabbing the top users, and then for each one going to the database and asking for their solution; it would be much better to use mongo's $in query to ask the question all in one go. There's also a limit argument you can pass to make sure you only get five results back, so that you won't need the take/filter approach.

Done. The querying was a little trickier than I expected, since I wanted to avoid showing solutions with missing content in this new "top users" section. The new code fetches 15 top users and picks 5 of their solutions which have content. We might need to bump up the original fetch amount if it turns out that most of the top users finished the majority of the problems before solutions were stored.

The history of these commits is also quite messy.

I updated the request with a cleaner set of two squashed commits.

@amalloy amalloy commented on an outdated diff Mar 28, 2013

src/foreclojure/problems.clj
(defn show-solutions [id]
(let [problem-id (Integer. id)
user (session/get :user)]
(if-user [{:keys [solved]}]
- (if (some #{problem-id} solved)
- (show-solutions-page problem-id)
- (flash-error (str "/problem/" problem-id)
- "You must solve this problem before you can see others' solutions!"))
- (do
- (session/put! :login-to *url*)
- (flash-error "/login" "You must log in to see solutions!")))))
+ (if (some #{problem-id} solved)
+ (show-solutions-page problem-id)
+ (flash-error (str "/problem/" problem-id)
+ "You must solve this problem before you can see others' solutions!"))
+ (do
+ (session/put! :login-to *url*)
+ (flash-error "/login" "You must log in to see solutions!")))))
@amalloy

amalloy Mar 28, 2013

Owner

I don't see a meaningful change here. Is this an accidental reindent, or am I missing something?

Contributor

hans commented Mar 28, 2013

Just removed the accidental indentation changes.

@amalloy amalloy added a commit that referenced this pull request Apr 11, 2013

@amalloy amalloy Merge pull request #235 from hans/develop
Show solutions from top users on problem solutions page
df813ac

@amalloy amalloy merged commit df813ac into 4clojure:develop Apr 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment