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

Starting to reformat loops into a unified concept #2773

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

manumafe98
Copy link
Contributor

@manumafe98 manumafe98 commented Apr 4, 2024

pull request

This PR reworks our current loop structure (for and for-each) into one big single concept called loops and also introduces while and do-while loops

closes #2625

Reviewer Resources:

Track Policies

@manumafe98 manumafe98 added the x:size/large Large amount of work label Apr 4, 2024
@manumafe98 manumafe98 self-assigned this Apr 4, 2024
@manumafe98 manumafe98 marked this pull request as draft April 4, 2024 19:05
@manumafe98 manumafe98 marked this pull request as ready for review April 5, 2024 20:41
@sanderploegsma
Copy link
Contributor

I'm aiming to review this early next week, FYI 😉

concepts/loops/about.md Outdated Show resolved Hide resolved
concepts/loops/about.md Outdated Show resolved Hide resolved
concepts/loops/about.md Outdated Show resolved Hide resolved
concepts/loops/about.md Outdated Show resolved Hide resolved
concepts/loops/introduction.md Outdated Show resolved Hide resolved
exercises/concept/making-the-grade/.meta/config.json Outdated Show resolved Hide resolved
exercises/concept/making-the-grade/.docs/hints.md Outdated Show resolved Hide resolved
exercises/concept/making-the-grade/.docs/instructions.md Outdated Show resolved Hide resolved

## Out of scope

- Specific iteration over a `Map`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this listed here? When I think about loops I don't necessarily think of maps.

Things that are related and seem to be out of scope of this exercise:

  • Breaking out of loops using the break keyword
  • Skipping iterations using the continue keyword

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of adding those to the out of scope I prefer mention them and maybe try to add exercise to cover those topics now that you say it, do you have in mind some other topics that could be out of scope? or I should remove it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be worried that also adding the control-flow statements to this exercise may make it a bit too big.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

6 tasks in a not very complex concept for me seems acceptable

concepts/loops/about.md Show resolved Hide resolved
manumafe98 and others added 6 commits April 26, 2024 17:28
Co-authored-by: Sander Ploegsma <[email protected]>
…pics were introduced

Adding links to the corresponding headings topics
Adding a different introduction instead of copying the about.md
Adding continue and break statements explanation and exercises
concepts/loops/about.md Outdated Show resolved Hide resolved
concepts/loops/about.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Whoa, I know I said to trim down the contents of the introduction but I think we need to give at least some information 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤣 What do you suggest to add then ? hahaha

"arrays",
"for-loops",
"foreach-loops"
"arrays"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe, I don't know to be honest. That would mean that the exercise is limited to just array creation and array indexing?

I'd like to have a good plan going forward, discussions like these become a bit too big for code reviews tbh.


## Out of scope

- Specific iteration over a `Map`
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be worried that also adding the control-flow statements to this exercise may make it a bit too big.

Comment on lines +71 to +82
static int evaluateChallengingExam(List<Integer> studentScores) {
int count = 0;

for (int score : studentScores) {
if (score <= 40) {
break;
}

count++;
}

return count;
Copy link
Contributor

Choose a reason for hiding this comment

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

This actually makes for a nice while task:

class MakingTheGrade {

    static int evaluateChallengingExam(List<Integer> studentScores) {
        int index = 0;

        while (studentScores.get(index) > 40) {
            index++;
        }

        return index + 1;
    }
}

😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you suggest to change this one for the one that is actually the while task? and make a new one for break ?

Copy link
Member

Choose a reason for hiding this comment

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

Floating another idea for trying to keep this as a break task, it might help to require them to something a bit more with the matching score. For example, if we require them to add the score to a second list (like keeping a list of the first failing score), the solution might look like this:

static void evaluateChallengingExam(List<Integer> studentScores, List<Integer> firstFailingScores) {
  for (int score : studentScores) {
    if (score <= 40) {
      firstFailingScores.add(score);
      break;
    }
  }
}

This way, we can even come up with tests for when the none of the scores pass.

manumafe98 and others added 3 commits May 7, 2024 12:03
Co-authored-by: Sander Ploegsma <[email protected]>
…l statements section

Adding two tests for task 5 and 6
Updating design.md to remove out of scope and add new missing learning objective
@manumafe98
Copy link
Contributor Author

Hey @kahgoh Do you mind helping me review this PR? Sander is taking a break from exercism so I will like your opinions on this topics

@kahgoh
Copy link
Member

kahgoh commented Jul 12, 2024

Hey @manumafe98, I'd be happy to help :) I hope to have a look at it within a day or two.

@manumafe98
Copy link
Contributor Author

Hey @manumafe98, I'd be happy to help :) I hope to have a look at it within a day or two.

Thanks! no rush


for (int score : studentScores) {
if (score % 2 == 0) {
continue;
Copy link
Member

@kahgoh kahgoh Jul 14, 2024

Choose a reason for hiding this comment

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

I notice the design says to have an essential comment if the student doesn't use continue. I think another valid way to solve this one is to add only odd scores in an if statements (i.e. no usage of continue):

static int countOddScores(List<Integer> studentScores) {
    int count = 0;

    for (int score : studentScores) {
        if (score % 2 != 0) {
            count++;
        }
    }
}

We could try to make up a more complicated calculation (although, haven't thought how it could fit with the story yet). For example, we could try something like:

  • Lets say we are trying to calculate a value. The value starts at 0. Then for each score ...
  • If the number is even, go to the next number (don't do anything to value).
  • Then, if the number is also greater than 20, add 5 to value.
  • Then, if the number is also greater than 60, then double the value.
  • Then, if the number is also greater than 80, subtract 50 from value.

We could even add something like "stop adding to the value if the number is greater than 90" to sneak in a possible break.

Comment on lines +22 to +25
while (count < numberOfStudents) {
sum += studentScores.get(count);
count++;
}
Copy link
Member

Choose a reason for hiding this comment

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

I think I would also likely use the for loop too (as per @sanderploegsma's comment). Giving them a list to go through "opens the door" for the for loop. Perhaps we could try moving away from a list - we could try ask them to call a method that returns something until a condition is met.

One way that came to my mind is that we could extend the challenging exam scenario by saying we want to see how many attempts it takes for a student to pass on exam. Then we could give them an Attempt object with a score() method that returns different a value each time it is called.

Comment on lines +71 to +82
static int evaluateChallengingExam(List<Integer> studentScores) {
int count = 0;

for (int score : studentScores) {
if (score <= 40) {
break;
}

count++;
}

return count;
Copy link
Member

Choose a reason for hiding this comment

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

Floating another idea for trying to keep this as a break task, it might help to require them to something a bit more with the matching score. For example, if we require them to add the score to a second list (like keeping a list of the first failing score), the solution might look like this:

static void evaluateChallengingExam(List<Integer> studentScores, List<Integer> firstFailingScores) {
  for (int score : studentScores) {
    if (score <= 40) {
      firstFailingScores.add(score);
      break;
    }
  }
}

This way, we can even come up with tests for when the none of the scores pass.

exercises/concept/making-the-grade/.docs/instructions.md Outdated Show resolved Hide resolved
@josealonso
Copy link
Contributor

@manumafe98, @kahgoh, happy to work with you on this one, guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:size/large Large amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reformat concept Loops
4 participants