aibolit icon indicating copy to clipboard operation
aibolit copied to clipboard

Clarify Nested Blocks pattern

Open Vitaly-Protasov opened this issue 3 years ago • 6 comments

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?

Vitaly-Protasov avatar Jul 23 '20 08:07 Vitaly-Protasov

seems we need

lyriccoder avatar Jul 23 '20 08:07 lyriccoder

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.

aravij avatar Jul 23 '20 08:07 aravij

@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 avatar Jul 23 '20 09:07 acheshkov

@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 avatar Jul 23 '20 09:07 acheshkov

I suggest keeping the current functionality.

acheshkov avatar Jul 23 '20 09:07 acheshkov

@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 avatar Jul 27 '20 18:07 aravij