Open RuslanLikhobaba opened 1 year ago
Hi, Ruslan
Thanks for the review. I will fix the points from the review. I have probably misunderstood some of the requirements, but it is easier now when I have an image to see how it should look at the end.
I will notify you when I finish the fixes.
@RuslanLikhobaba
I have just pushed my new code. You can check it.
Please contact me if you have any questions.
Best regards
Changes:
Issues:
Hi Nikola, Good job, your logic works as expected.
Changing input to content-editable div created a bug about a cursor that moves to the beginning of the input area
Don't worry about it. We know about this one and don't expect perfect code here.
I would like to draw your attention to a potential memory leak in your code. For testing, you can make these changes to the code:
app.component.ts
export class AppComponent {
...
isVisible = false;
}
app.component.html
<button (click)="isVisible = !isVisible">{{isVisible ? 'Hide Block' : 'Show Block'}}</button>
<app-text-input *ngIf="isVisible"/>
text-input.component.ts
ngOnInit() {
interval(2000)
//this.debounceTrigger
.pipe(debounceTime(1000))
.subscribe((i) => {
console.log('message', i);
this.handleHighlights();
});
}
Play around with the display of the block and at the same time watch for messages in the browser console.
Hi @RuslanLikhobaba,
I have added a subscription and unsubscribed from it in the onDestroy hook. Also, I have managed to create a logic for the CURSOR issue from yesterday, so now everything seems perfect :)
Best regards
Good evening @nikola87kg,
I see you are not afraid to spend time on an interesting task. I have already left my positive opinion this morning about your work.
Just for fun, I suggest you to think about the complexity of the algorithm you implemented. Also how the algorithm will change if you need to find not only the correct patterns, but also the wrong ones, that is, those patterns that intersect.
Motivation: in my opinion, this algorithm has a weak point, these are regular expressions. It doesn't matter for single searches of the same type. But when pattern searching is done in real time while typing, this can become a bottleneck. When looking for the wrong patterns, how many more regular expressions will you have to add?
Best regards
Hi @RuslanLikhobaba,
I have improved the algorithm and removed regex completely. Now, all the checking is done in one for loop, and I made it scalable. I have tested it with the third colour (red) and it works fine. I left a sample in the test.
Best regards, Nikola
Hi @nikola87kg,
Yes! It' possible to find all patterns in one loop without using regular expressions. It doesn't depend on me much, but I'd like to work together.
Best regards
Hi Nikola, Thank you for taking the time to complete the test task.
The task assumed that it would look like an editable block, but it is more important for us to see your approach to work. The current your implementation does not implement some of the requirements of the task.
This is the checking your imlementation: These patterns are incorrect: {{test}} [[test]] {te[s]t} [te{s}t] {te[s}t].
You can fix it and we will review the code again.
I also have feedback on some of the points that won't be rated but make me curious:
handleHighlights
will be called for every keypress event. Perhaps you should consider using the debounce operator.greenRegex
andblueRegex
will create newRegExp
objects for every keypress event, but they have constant params. Perhaps you should consider making them private properties of the component.The test task is taken as a general option from the typical tasks of our project, so we can show it on a real example
Best regards