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

Adding analyzer feedback for karls language concept exercise #2722

Conversation

manumafe98
Copy link
Contributor

pull request

closes #2678

To be consistent with lasagna design.md, I made a couple of changes, so instead of using a loop make the user reuse code seem better for me.

Reviewer Resources:

Track Policies

@manumafe98
Copy link
Contributor Author

manumafe98 commented Feb 5, 2024

I have also a couple of questions:

  • maybe the method called isEmpty is literally giving the solution to the student, maybe we should rename it?
  • Also seeing the hints file, and checking other files we are using documentation of different versions on java, we don't have a standard, I was thinking that we could maybe create a variable and assign it to the latest version, so the documentation that the student has is always updated? I'm open to ideas

@manumafe98
Copy link
Contributor Author

If we 100% want to make for-loops a prerequisite we could replace the last method isExciting that currently for me seems pointless for something like this

    public <T> String isInstanceOf(List<T> list) {
        for (T element : list) {
            if (element instanceof Integer) {
                return "Integer";
            }
        }
        return "String";
    }

And introduce the concept instanceof

Copy link
Contributor

Choose a reason for hiding this comment

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

I think these hints may give away a bit too much. Based on the documentation I'd say they could be just a little bit more cryptic. For example, instead of

Try using the isEmpty() method.

it might say

One of the methods on the List type can be used to check if it is empty.

Same for the other hints.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

exercises/concept/karls-languages/.meta/design.md Outdated Show resolved Hide resolved
@sanderploegsma
Copy link
Contributor

Also seeing the hints file, and checking other files we are using documentation of different versions on java, we don't have a standard, I was thinking that we could maybe create a variable and assign it to the latest version, so the documentation that the student has is always updated? I'm open to ideas

How would that work exactly? AFAIK we can't define variables in Markdown files.

@sanderploegsma
Copy link
Contributor

If we 100% want to make for-loops a prerequisite we could replace the last method isExciting that currently for me seems pointless for something like this

    public <T> String isInstanceOf(List<T> list) {
        for (T element : list) {
            if (element instanceof Integer) {
                return "Integer";
            }
        }
        return "String";
    }

And introduce the concept instanceof

Nah I think removing the for-loops prerequisite is the way to go here, the concept is really not needed for this exercise. Also based on the community solutions almost nobody uses a loop for the last task.

Update hints to not say exactly what to do
Update isEmpty name to one that does not says directly what to apply
change the analyzer topic that checks that the students re uses containsLanguage to informative
Add an informative topic if the students uses an if statement on containsLanguage
@manumafe98
Copy link
Contributor Author

How would that work exactly? AFAIK we can't define variables in Markdown files.

Yeah, I thought that the variables accept plain text, but it seems that they only accept URLs. However, I still think we should find a way to handle this, because currently, we have a ton of documentation for different Java versions – 7, 8, 9, 11, 12, 13, 14, etc. Perhaps a policy to only add documentation for the latest current version, but that wouldn't eliminate the need to update the older ones at some point.

@manumafe98 manumafe98 self-assigned this Feb 7, 2024
@manumafe98 manumafe98 added the x:size/small Small amount of work label Feb 7, 2024
@sanderploegsma
Copy link
Contributor

How would that work exactly? AFAIK we can't define variables in Markdown files.

Yeah, I thought that the variables accept plain text, but it seems that they only accept URLs. However, I still think we should find a way to handle this, because currently, we have a ton of documentation for different Java versions – 7, 8, 9, 11, 12, 13, 14, etc. Perhaps a policy to only add documentation for the latest current version, but that wouldn't eliminate the need to update the older ones at some point.

Can you add a new issue to discuss this?

@manumafe98
Copy link
Contributor Author

Can you add a new issue to discuss this?

Sure

@manumafe98 manumafe98 merged commit 832c703 into exercism:main Feb 8, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:size/small Small amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

karls-languages: describe Analyzer feedback in .meta/design.md
2 participants