-
Notifications
You must be signed in to change notification settings - Fork 520
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
✨ (concepts) Add concept exercise annalyns-infiltration
#1668
base: main
Are you sure you want to change the base?
Conversation
0577327
to
0312378
Compare
{ | ||
"slug": "annalyns-infiltration", | ||
"name": "Annalyn's Infiltration", | ||
"uuid": "897fa2ea-c29c-4d4e-9469-ff661a9b838a", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly copied csharp track and generated a uuid v4. Is there anything else that needs to be done to enable this concept?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately concept exercises just got disabled for Rust. The syllabus will need a complete rework, so it might take some time to enable this concept exercise. (CC @ErikSchierboom)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem, this is exactly why I'm implementing simpler concept exercises. I only wanted to know if there are any other config files to touch to prepare it for enabling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will show in-progress exercise for track maintainers though.
6ef3f8d
to
bcbc65c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good! Just a couple of comments
{ | ||
"slug": "annalyns-infiltration", | ||
"name": "Annalyn's Infiltration", | ||
"uuid": "897fa2ea-c29c-4d4e-9469-ff661a9b838a", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will show in-progress exercise for track maintainers though.
9550d96
to
f1d0cb6
Compare
_knight_is_awake: bool, | ||
_archer_is_awake: bool, | ||
_prisoner_is_awake: bool, | ||
_pet_dog_is_present: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is silencing the unused variable warning really desirable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been done this way for many exercises on the Rust track. I'm not a huge fan of it personally either. One way to suppress the warning to use it in the unimplemented
macro:
unimplemented!("use {knight_is_awake} to solve the exercise")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might be preferable. I expect many/most students will just use the names provided.
Also, showing that warning here might be good pedagogy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iHiD Is there an Exercism-wide policy here?
If (concept) exercise stubs are
- supposed to be free of warnings, then I propose using @remlse's workaround (track-wide).
- allowed to induce warnings, then I propose to just remove the initial underscores (track-wide) – possibly also for practice exercises.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have an Exercism-wide policy. I don't mind warnings personally, and in this case they can actually help the student as they make it clear that the student has to do something. I wonder if there is some CI running that will balk at warnings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's find out.
_knight_is_awake: bool, | |
_archer_is_awake: bool, | |
_prisoner_is_awake: bool, | |
_pet_dog_is_present: bool, | |
knight_is_awake: bool, | |
archer_is_awake: bool, | |
prisoner_is_awake: bool, | |
pet_dog_is_present: bool, |
79ba19e
to
be86eca
Compare
This concept is inspired from the same in csharp track
_knight_is_awake: bool, | ||
_archer_is_awake: bool, | ||
_prisoner_is_awake: bool, | ||
_pet_dog_is_present: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's find out.
_knight_is_awake: bool, | |
_archer_is_awake: bool, | |
_prisoner_is_awake: bool, | |
_pet_dog_is_present: bool, | |
knight_is_awake: bool, | |
archer_is_awake: bool, | |
prisoner_is_awake: bool, | |
pet_dog_is_present: bool, |
unimplemented!("Implement can_fast_attack"); | ||
} | ||
|
||
pub fn can_spy(_knight_is_awake: bool, _archer_is_awake: bool, _prisoner_is_awake: bool) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub fn can_spy(_knight_is_awake: bool, _archer_is_awake: bool, _prisoner_is_awake: bool) -> bool { | |
pub fn can_spy(knight_is_awake: bool, archer_is_awake: bool, prisoner_is_awake: bool) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is off-topic for this PR. Both approaches are currently being used on the track. I agree we should make this consistent and introduce some CI test to make sure it stays consistent, but this PR is not the place to do it. Let's keep the underscores for now.
unimplemented!("Implement can_spy"); | ||
} | ||
|
||
pub fn can_signal_prisoner(_archer_is_awake: bool, _prisoner_is_awake: bool) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub fn can_signal_prisoner(_archer_is_awake: bool, _prisoner_is_awake: bool) -> bool { | |
pub fn can_signal_prisoner(archer_is_awake: bool, prisoner_is_awake: bool) -> bool { |
@@ -0,0 +1,20 @@ | |||
pub fn can_fast_attack(_knight_is_awake: bool) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub fn can_fast_attack(_knight_is_awake: bool) -> bool { | |
pub fn can_fast_attack(knight_is_awake: bool) -> bool { |
@@ -19,18 +75,43 @@ values into a boolean value. | |||
|
|||
Unlike some other languages, `0` is not `false` and non-zero is not `true`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A student may wonder, what happens if a number value is used as a boolean? Is that a runtime panic?
Might be good to note here that it is a compile time error and the programmer is helpfully prevented from making this mistake.
@@ -19,18 +75,43 @@ values into a boolean value. | |||
|
|||
Unlike some other languages, `0` is not `false` and non-zero is not `true`. | |||
|
|||
Rust supports three [logical binary operators][logical binary operators]: `&` (Logical And), `|` (Logical Or), `^` (Logical Xor). | |||
Rust supports three [logical binary operators][logical binary operators]: `&` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"binary operator" usually means "an operator that takes two operators". (as opposed to unary and ternary operators).
I suggest "bitwise operator" as a less ambiguous term here.
Rust supports two [boolean operators][lazy boolean operators]: `||` (Or), `&&` (And). They differ from `|` and `&` in that the right-hand operand | ||
is only evaluated when the left-hand operand does not already determine the result of the expression. That is, `||` only evaluates | ||
its right-hand operand when the left-hand operand evaluates to `false`, and `&&` only when it evaluates to `true`. | ||
The operators `||` (OR), `&&` (AND) differ from `|` and `&`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we should omit any mention of the bitwise operators in the concept about booleans. I've only ever seen bitwise operators used with unsigned integer types, where they operate on bit patterns.
For booleans, as is correctly described here, the difference between |
and ||
etc. is very small - it's ony about the lazy evaluation. And a program which relies on lazy / eager evaluation of it's boolean expressions because the operands are side-effectful... is a terrible program in my opinion.
In other languages, it may be common to use the lazy boolean operators for control flow, but not in Rust.
With all that in mind, talking about the bitwise operators for booleans at all seems confusing at best, and suggestive of bad practices at worst.
Opinions? @MatthijsBlom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fully agree that some Operations on Bits concept node would be a better place for introducing the bitwise operators.
And a program which relies on lazy / eager evaluation of it's boolean expressions because the operands are side-effectful... is a terrible program in my opinion.
Not exactly a counterexample – because it does not involve side effects – but: e.g. if some_list and predicate(some_list[0])
is a fairly common construct in Python. The some_list
checks that it is not empty; only if it nonempty is some_list[0]
looked up, avoiding a runtime error.
More generally, successful evaluation of subsequent propositions might depend on prior propositions holding.
let f: bool = false; // with explicit type annotation | ||
``` | ||
|
||
The main way to use Boolean values is through conditionals, such as an if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the exact purpose of about.md
and introduction.md
on a concept? There is a lot of duplicated text between the two files, is that normal? I guess would be that some exaplantion should go in one of the two files - not in both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are documented here: https://exercism.org/docs/building/tracks/concepts
introduction.md
is shown to students who have not yet solved the concept exercise(s?), and about.md
is shown to those who have.
It is entirely normal for the two to share a lot of text. It also happens that they are identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's entirely correct.
let f: bool = false; // with explicit type annotation | ||
``` | ||
|
||
The main way to use Boolean values is through conditionals, such as an if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is even more duplication here. The text explaining booleans seems to be copied three times? in the concept's about, the concept's introduction and here in the exercise's introduction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is documented here: https://exercism.org/docs/building/tracks/concept-exercises#h-file-docs-introduction-md
|
||
- Know of the existence of the `bool` type and its two values. | ||
- Know about boolean operators and how to build logical expressions with them. | ||
- Know of the boolean operator precedence rules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, the concept introduction does not explain the precedence rules. Let's add that.
The author of this PR has stopped responding in the discussions planning the work on the syllabus. But there's been a lot of work put into reviews already, so I'm keeping it open in case it can be salvaged in a future attempt to create a good syllabus. |
For the record, I'd like to state that pacman-rules is usually the better choice to teach booleans and boolean expressions. |
This concept is inspired from the same in csharp track