Skip to content

Conversation

@EliasAAradsson
Copy link
Contributor

This is currently a draft in need of review.

Instead of implementing the TODOs, I got feeling and rewrote the subtasks b, c, and d, into one (in my opinion) better subtask. Currently the Latex is a bit off and in need of proper formatting, hence this PR is to be considered a draft. If the change is appreciated I would like some guidance on how to make the Latex nice and as the rest of the compendium.

@bjornregnell
Copy link
Member

bjornregnell commented Oct 20, 2024

Thanks for working on this. What is the goal of the change, i.e. why is it an improvement? Is it because of lazy vals instead? But I guess lazy vals are not always an improvement... Was there any problems that you observed by studentes trying to solve it? Also I guess the task needs to say something about the requirements on what should be implemented. And perhaps the names of the proposed new methods could be improved?

@EliasAAradsson
Copy link
Contributor Author

EliasAAradsson commented Oct 21, 2024

Thanks for working on this. What is the goal of the change, i.e. why is it an improvement?

My idea was that the new b-task ties together better with the (modified) a-task. All members defined in the a-task now have a clear place in the solution to the b-task. Also I thought that the old tasks b, c, and d, weren't that clearly tied to the goal of w05, i.e. data modelling with classes. They were more about just using the results from a to write functions, which can be a useful exercise, but I thought focus could be moved towards writing a class representing a polygon. Moreover the d-task really doesn't have a straight answer and is perhaps not so relevant to the material meaning I thought it could be removed.

Another thought was to add some given code (the PolygonWindow object) to train students further in interacting with a given block of code (since the task is a part of the "extra" section in w05 leading up to blockbatte, which really is the first lab with a substantial amount of given code).

Is it because of lazy vals instead? But I guess lazy vals are not always an improvement...

That was just some extra spice. Since lazy vals are used in the w05 theory I thought I could put them there, further exposing students to the idea of not calculating values that perhaps won't be used. If you believe they would be confusing to students feel free to remove them.

Was there any problems that you observed by studentes trying to solve it?

Well, some indication that task c feels misplaced. a relates to b since polar coordinates are useful for doing "circular stuff" and d is a reflection on this. c is just connecting x1 to x2 and y1 to y2. So even if you don't like my rewrite it could still be relevant to switch the order of tasks c and d.

Also I guess the task needs to say something about the requirements on what should be implemented. And perhaps the names of the proposed new methods could be improved?

Absolutely, it's not just the Latex that's in need of clean up. Some proper instructions will be needed, but if my proposed change to the exercise isn't considered an improvement, there is no point in writing good Swedish to accompany it, hence I haven't done so yet. And names can always be discussed.

So in conclusion, and as I said, it's a draft and just something I did for fun, and as a suggestion. No hard feelings at all if you just close the PR without further comments! (With that said, it would be interesting to hear about why of course.) (^‿^)

@bjornregnell
Copy link
Member

OK fine - thanks for detailed explanations. Sounds like a good improvement; please go ahead as you please and let me know when I can help or provide a review.

@EliasAAradsson
Copy link
Contributor Author

EliasAAradsson commented Oct 29, 2024

OK fine - thanks for detailed explanations. Sounds like a good improvement; please go ahead as you please and let me know when I can help or provide a review.

Now I have written a cleaner improved version, please review. I removed the lazy stuff as well; I think you're right in that it takes focus from the rest and leads into a discussion about whether or not lazy is good in this particular situation etc., which feels off topic for the exercise. Latex is still bad, but the rest I think is okay for merging.

@bjornregnell
Copy link
Member

LGTM! What do you mean "latex is still bad"?

@bjornregnell bjornregnell merged commit 8f40f97 into lunduniversity:master Oct 31, 2024
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

Successfully merging this pull request may close these issues.

2 participants