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

Clarify Nested Blocks pattern #562

Open
Vitaly-Protasov opened this issue Jul 23, 2020 · 6 comments
Open

Clarify Nested Blocks pattern #562

Vitaly-Protasov opened this issue Jul 23, 2020 · 6 comments
Assignees
Labels
discussion There is something to discuss, before code

Comments

@Vitaly-Protasov
Copy link
Collaborator

Vitaly-Protasov commented Jul 23, 2020

Once I started to refactor 'Nested Blocks pattern', I faced the misunderstanding of how it should works.

First of all, I found block_type argument is undue one, because I think that we do not need to specify the type of statement which we want to search for. I think it is better to find all nested blocks inside each other.

To be more clear, let's consider the example:

class Test {
    public void start() {
        for (int i = 0; i < 10; i++)
            for (int i = 0; i < 10; i++)
                list.add(Boolean.FALSE);

        for (int i = 0; i < 10; i++)
            for (int i = 0; i < 10; i++)
                list.add(Boolean.FALSE);
        
		if (a > b) {
			if (a > c) {
			return a
			}
		return c}
    }
}

According to the previous implementation, this pattern only can find 1 string of nested block ( whether it is FOR or IF).

I propose to reimplement it and count 2 string of nested block (both FOR and IF).

Also, could you explain, should we consider this pattern, as for the code snippet below:

class Test {
    public void start() {
        for (int i = 0; i < 10; i++)
		if (i > 20)
			for (int i = 0; i < 10; i++)
				list.add(Boolean.FALSE);
    }
}

Here we have nested for->if->for. Is it nested block or not?

The main question. nested block is nested only into the block with the same type?

@aravij aravij added the discussion There is something to discuss, before code label Jul 23, 2020
@lyriccoder
Copy link
Member

seems we need

@aravij
Copy link
Contributor

aravij commented Jul 23, 2020

I am for finding out nested blocks with for, if etc. at once.

Considering nested blocks of different types, their nesting depth should be greater than for nested blocks of same type. For example triple nested if is definitely complex structure to me, but nested for->if->for is arguable.

@acheshkov
Copy link
Member

acheshkov commented Jul 23, 2020

@Vitaly-Protasov

I propose to reimplement it and count 2 string of nested block (both FOR and IF).

When this pattern was suggested, It supposed to match only the nested STATEMENTS of the same kind.

@acheshkov
Copy link
Member

@Vitaly-Protasov
The current implementation has the following features

  1. Counts nested STATEMENTS of the kinds from the list parameter block_type
  2. The length of nesting is the parameter max_depth

for example, If block_type=['DO', 'WHILE'] and max_depth=3 the we have to match the following nestied combinations:

[DO, DO, DO]
[WHILE, WHILE, WHILE]
[DO, WHILE, WHILE]
[WHILE, DO, WHILE]
[WHILE, WHILE, DO]

...

and so on.

@acheshkov
Copy link
Member

I suggest keeping the current functionality.

@aravij
Copy link
Contributor

aravij commented Jul 27, 2020

@Vitaly-Protasov So to keep current functionality, pattern class needs to accept in its constructor max_depth and tuple of statements types. It is better be done the following way:

def __init__(self, max_depth: int, *block_types: ASTNodeType):
    assert len(block_types) > 0, "Please provide at least single type"
    ...

Also notice, that there is pattern nested_loop. It is actually the same pattern, just with some predefined configuration. We better stick with single pattern, and provide one of possible configuration with config.py.

If everything is clear, you may close the issue.

@aravij aravij mentioned this issue Jul 27, 2020
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion There is something to discuss, before code
Projects
None yet
Development

No branches or pull requests

4 participants