2023-02-06 14-27-11
André Smit
25Views
9Comments
Solved
[AI Mentor] Seeking Assistance with Code Duplication in Application
Question
AI Mentor Studio
Best Practices
Development

Hi all,

I hope this message finds you all in good health. I am currently in the process of working through our application alongside the AI Mentor, focusing on necessary fixes and enhancements. However, I've hit a roadblock while addressing duplicated code that requires your expertise and insights.

To aid in conveying the issue effectively, I have attached an OML demo that visually represents the challenge I'm facing. The concern lies in the presence of identical code segments within both the Projects and Invoices screens. While I understand the nature of code repetition, I'm seeking guidance on the most effective approach to consolidate these segments into a single client action that can seamlessly serve both screens. I believe that by doing so, I can effectively resolve the current issue.

I would greatly appreciate your valuable input and experience in tackling such scenarios. If any of you have encountered similar situations or can offer suggestions on streamlining and optimizing this code, your assistance would be invaluable to me.

Thank you very much for your time and consideration.


zGenericFilter.oml
0
0
31 Aug
Solution

Hi André,

I've looked at the OML and my first idea would be to create a WebBlock for the filters. That way you can centralize the "FilterStatus" and via event propagation you can trigger the refreshes with the correct filter value in the handlers on the respective screens.

So it would go somewhat like this:

User selects tab, this will trigger an event in the WB that will fire to the parent which tab has been pressed and based on the switch logic (which will happen in a client action in the WB) it will return the correct filter and in the handler you assign that filter and refresh the aggregate.

My second suggestion is to make a function that you can call with two input parameters:

1. The clicked tab

2. The screen/type the call originates from

Based on those two inputs you can return the correct filter and you can handle this all including the refresh in the OnTabChange event from the 'Navigation\Tabs' widget. If you go for this option I would advise to split the function into two layers, the top function which is called from the screen has a switch just on type lvl (so parameter 2) and then it sends the call to a client action that contains the specific switch for the tab value. That way it is more maintainable.

Kind regards,

Bart

See solution in context
1
0
31 Aug
Solution

Yes,

1) your list-building of the header list is indeed what i meant.

One remark about it, I would, for simplicity, do the pre-appending with the All option in the server, so the DataAction returns the complete and correct list.

=> no need for the PopulateTabHeaders action, no ListClear, no need for a local 'HeaderList' variable.  

You then can directly use as source for the list widget inside your tabs header the list being returned from the DataAction.  But that's largely a matter of taste, I personally always like to reduce the number of moving parts.


2) As for applying the selected tab to the filter, you are now referencing the HeaderList.Current.Id, but current is not affected by choosing a tab.  You'll have to reinstate the OnTabChange of the tabs widget, and capture the id of the selected tab.  This can not be done with 'Current', but by indexing into the list.

So instead of

HeaderList.Current.Id

it is

HeaderList[ActiveTab].Id

Set the value of that to an extra local variable with type ProjectStatusId, then do a refresh and use that local variable as filter.

I amended your OML.  


3) The caution about the widget not being meant for this was only that, a caution :  sometimes this can lead to unpredictable behaviour because the makers of the widget (Outsystems team) don't take your use case in consideration when they update the widget.

Dorine

See solution in context
zGenericFilter+list amended.oml
0
0
05 Sep
Solution

Hi André,

I've looked at the OML and my first idea would be to create a WebBlock for the filters. That way you can centralize the "FilterStatus" and via event propagation you can trigger the refreshes with the correct filter value in the handlers on the respective screens.

So it would go somewhat like this:

User selects tab, this will trigger an event in the WB that will fire to the parent which tab has been pressed and based on the switch logic (which will happen in a client action in the WB) it will return the correct filter and in the handler you assign that filter and refresh the aggregate.

My second suggestion is to make a function that you can call with two input parameters:

1. The clicked tab

2. The screen/type the call originates from

Based on those two inputs you can return the correct filter and you can handle this all including the refresh in the OnTabChange event from the 'Navigation\Tabs' widget. If you go for this option I would advise to split the function into two layers, the top function which is called from the screen has a switch just on type lvl (so parameter 2) and then it sends the call to a client action that contains the specific switch for the tab value. That way it is more maintainable.

Kind regards,

Bart

1
0
31 Aug

@Bart van Orizande 

I've taken your guidance for inspiration, and I believe I've arrived at a solution.

I've developed a 'GenericFilter' client action with three parameters:

  • CurrentTab (input)
  • Screen (input)
  • QuickFilter (output)

The CurrentTab parameter determines the currently selected tab, the Screen parameter identifies the active screen for the user, and QuickFilter will be utilized in the aggregate for filtering.

I'm uncertain if AI Mentor will still flag duplicate code, as both these actions technically follow the same pattern. However, I'll assess this after it has been synced again.

zGenericFilter+refined.oml
0
0
01 Sep

Hi André,

That sounds like a good idea! One thing that is worth thinking about is to not have it on a screen level because screens can change over time. It's better to determine a typing of some sort and stick to that. 

If my message helped you, would you mind marking it as the answer, that way it pops up easily for people searching for the same :D

0
0
01 Sep

Replying to Bart van Orizande's comment on 01 Sep 2023 15:12:06


Hello Bart,

I'm sorry for any confusion, but please clarify what you mean by "determine a typing of some sort" and why it shouldn't be on the screen level. You might be suggesting a more practical approach, and I'd appreciate your insights.

Thank you for taking the time to explain.

0
0
04 Sep

I'd like to broaden this conversation a little bit :

I think the real duplication here, is that you twice define a list of what constitutes a possible project status or what constitutes a possible invoice status, you already define that as a static entity, and instead of making use of that, you re-define that in a hardcoded way in the tabs being placed on the screen, and by having a switch with all the possible filter values. (so actually not 2 but 3 separate parts of the application that need to be kept in sync)

And also related technical debt I see : although you have your static entities available to make use of, you use as a filter in the aggregate, the label instead of the id.  Labels can change over time, and you'll have to take care to also review your hardcoded condititons and the hardcoded tab texts whenever they do.

I know this is maybe just a 'low hanging fruit' kind of exercise for your right now, but the real improvement would come from

  • building the list of possible tabs based on the records in the static entity instead of hardcoded
  • registering at every tab change, what the static entity id is that was selected
  • apply that id in your filter instead of the label

Voila, no need for any swith  anymore.

Depending on whether you have this kind of QuickFilter in many screens or not, it might be interesting to make a generic block for it, but for just 2 screens, I wouldn't bother.

Dorine

PS. a completely unrelated remark, I don't think the tabs widget is intended to be used with headers only as a filter feature, but if it is working for you, ok then.

0
0
04 Sep

just a little extra tip : if you go the route of using a list to populate your tabs header, you need to give it an extra property to make the tabs widget work well with it 

0
0
04 Sep

Ofcourse,

As Dorine mentioned you have two different lists as statusses. Now I don't know the background of that decision but from what I could see it is currently only being used differently depending on the screen. That is also why you need a screen as input.

My suggestion was to create the list based on different typing instead of a screen. To demonstrate picture the following:

I have a screen on which I show the bookings of a hotel, those can be open, reserved and booked. Now I can have those statusses for that screen, but that screen might change in such a way that I dont need this functionality anymore. Also when I need those same statusses somewhere else I need to add this screen to the switch etc.

If I determine that the typing is "booking" or something along those lines I can use it for every screen that needs the booking statusses and even if the screen changes it doesn't mean that the block has to change because the typing will remain viable to pick when you need booking statusses.

As for Dorine's suggestion, I think it's a very good idea to implement the switch into the tab, you should definitely consider doing that.

0
0
04 Sep

Thank you for your always detailed responses, @Dorine Boudry. I truly appreciate it. 

I believe I'm grasping your point correctly. The list-building process appears to have been successful, as the tabs now display the correct labels, including an 'All' option to show all the data. I attempted to implement your solution on the 'Projects' screen, but I'm encountering challenges with the registration process at every tab change. Please refer to the attached OML for more context. On a positive note, your suggestion regarding 'disable-virtualization' for the list seems to be working flawlessly.

Regarding the use of tabs for filtering, it has been working well for me, and I haven't noticed any issues with performance. If there's a better pattern you'd recommend instead, I'd be open to exploring it.

@Bart van Orizande, I must acknowledge that both of you have considerably more experience with OutSystems than I do. Some of the details you've discussed have gone a bit over my head, to be frank, but I appreciate your feedback.

zGenericFilter+list.oml
0
0
05 Sep
Solution

Yes,

1) your list-building of the header list is indeed what i meant.

One remark about it, I would, for simplicity, do the pre-appending with the All option in the server, so the DataAction returns the complete and correct list.

=> no need for the PopulateTabHeaders action, no ListClear, no need for a local 'HeaderList' variable.  

You then can directly use as source for the list widget inside your tabs header the list being returned from the DataAction.  But that's largely a matter of taste, I personally always like to reduce the number of moving parts.


2) As for applying the selected tab to the filter, you are now referencing the HeaderList.Current.Id, but current is not affected by choosing a tab.  You'll have to reinstate the OnTabChange of the tabs widget, and capture the id of the selected tab.  This can not be done with 'Current', but by indexing into the list.

So instead of

HeaderList.Current.Id

it is

HeaderList[ActiveTab].Id

Set the value of that to an extra local variable with type ProjectStatusId, then do a refresh and use that local variable as filter.

I amended your OML.  


3) The caution about the widget not being meant for this was only that, a caution :  sometimes this can lead to unpredictable behaviour because the makers of the widget (Outsystems team) don't take your use case in consideration when they update the widget.

Dorine

zGenericFilter+list amended.oml
0
0
05 Sep
Community GuidelinesBe kind and respectful, give credit to the original source of content, and search for duplicates before posting.