Skip to content
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

Add leap #72

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Add leap #72

merged 1 commit into from
Nov 28, 2024

Conversation

BNAndras
Copy link
Member

No description provided.

Comment on lines +4 to +6
set "year=%~1"
set "result="

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a practice exercise, the tests are meant to be read, can these three lines be gone?

If not, is line 5 valid? If not, we should note deliver code that is inherently broken on its own to students. We might assign it to a number, or we might assign it to a place holder string such as something that says "replace this with a good default value", but it should not be broken on its own, delivered.

Copy link
Member Author

@BNAndras BNAndras Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree about line 5, but I used the darts implementation here as a guide. That’s how the maintainer did it there. The only change I did was to show the test name for passing tests since the test name is shown only on failure for the other exercises.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not really feel strongly about it either way, but if these are practice I have been noticing some students taking some of the "scaffolding" that is sometimes delivered as "given" and "not to be touched". And unless told explicitly otherwise, I would want the practice exercises to provide all the areas to practice, where it is not something that "Must be done and this way and even by name, generally."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the extent even that the "default year" is set? What is a default year, and how did we choose that?"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not following about the default. year is being set to the input for each test being checked. lt’s above the line telling the student to add their code so students just need to know that variable contains the test input. Would it address some of the potential confusion for me to rename that variable as input?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kotp, may I don't get what is wrong, 'year' is a parameter and %~1 means it's the first parameter, this exercise takes one parameter so we don't need to add the second parameter, and the result added to that can be updated so at the end of the program that will be printed and unit tests able to see the output. In the code part set "result=" is not affecting program, that means, at the start we don't have a variable that results, and set "result=" removes the variable from memory, so can't remove a variable that doesn't initialize. In the batch file, we no need to initialize variables, when you try to assign, if not has that variable, creates one of them, so open to manipulation at the center of code.

@GroophyLifefor GroophyLifefor self-requested a review November 28, 2024 15:11
Copy link
Member

@GroophyLifefor GroophyLifefor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing all unit tests, thank you for your contribute ❤️

@BNAndras BNAndras merged commit 8b1a9cb into exercism:main Nov 28, 2024
4 checks passed
@BNAndras BNAndras deleted the add-leap branch November 28, 2024 18:44
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.

3 participants