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

Style50 counts header comments as lines of code, but not as comments #76

Open
Jelleas opened this issue Sep 7, 2018 · 6 comments
Open

Comments

@Jelleas
Copy link

Jelleas commented Sep 7, 2018

This overly commented snippet of code needs more comments according to style50.

/******************************
 *
 * hello.c
 *
 * This script prints "Hello, world! in the terminal window.
 *
 * Created by: (UvA Student)
 *
 ******************************/

// Including needed header file
#include <stdio.h>

// The main function, which will be started automatically.
int main(void)
{
    // Print "Hello, World!" in the terminal window.
    printf("Hello, world!\n");
}
~/workspace/ $ style50 hello.c
Looks good!
But consider adding more comments!

Apparently removing the header comment fixes the issue?

// Including needed header file
#include <stdio.h>

// The main function, which will be started automatically.
int main(void)
{
    // Print "Hello, World!" in the terminal window.
    printf("Hello, world!\n");
}
~/workspace/ $ style50 hello.c
Looks good!
@cmlsharp
Copy link
Contributor

cmlsharp commented Sep 27, 2018

Ooh actually this is sort of two issues in one. The way style50 counts comments (for C anyway) is by first removing all the string literals, and then matching what remains against a regex. This is to prevent something like printf("// this isn't a comment") from being counted. However, this can have some pretty bad behavior if you have just a single " in your comment. (as you do in This script prints "Hello, world! in the terminal window.). Then when it counts comments, it removes what it sees as the string literal

"Hello, world! in the terminal window.
 *
 * Created by: (UvA Student)
 *
 ******************************/

// Including needed header file
#include <stdio.h>

// The main function, which will be started automatically.
int main(void)
{
    // Print "Hello, World!" in the terminal window.
    printf("

Regexes are probably ill-suited for this task. I may just need to use a C parser

@charbelsako
Copy link

Encountered a problem where the program said to remove 4 spaces from two line of code. However, removing those spaces would make the code worse.

@Rohitth007
Copy link

@cmlsharp Suppose the single " wasn't there, wouldn't it still count 9 lines of header comments as 1 comment decreasing the ratio?

@cmlsharp
Copy link
Contributor

cmlsharp commented Oct 25, 2020

@Rohitth007 yep, that's a flaw for sure. It doesn't impact grading at all, but there are cases in which writing a multi-line comment as opposed to a single line comment would cause the warning to be displayed. For example:

/* foo
 * bar */
int main(void)
{
    foo();
    foo();
    foo();
    foo();
    foo();
    foo();
}

yields the "But consider adding more comments" message but

/* foo bar */
int main(void)
{
    foo();
    foo();
    foo();
    foo();
    foo();
    foo();
}

does not.

This could be fixed by having the C StyleCheck class override count_lines to count multi-line comments as a single line.

@cmlsharp
Copy link
Contributor

The right solution to this and the original issue is to use a lexer like Pygments to implement these normalization steps (ideally in a language agnostic way instead of leaving count_comments and count_lines up to the children of StyleCheck).

@Rohitth007
Copy link

@cmlsharp Instead of counting a multi-line comment as a single line, isn't counting an n-line comment as n comments, by changing count_comments, a better solution? 2/11 over 1/10 in your example.

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

No branches or pull requests

4 participants