Maker of the Month Challenge July 2020

9
Hello, Makers!   We’re excited to announce the Maker of the Month challenge of July. What is the challenge you might wonder? We'll challenge you with a new challenge based on the new release every month, the perfect opportunity to test and to show off your Mendix skills. Whoever provides the most detailed answer with the best explanation, receives a free fancy Mendix t-shirt and a ‘Maker of the Month’ badge on your developer's profile.      Challenge: Time to shine! Show off your microflow best practice skills with the provided use-case.     You have been assigned the task of performing a code review on your teammate. Your teammate was tasked with building a notification email for completed orders, to notify the customer that their order is complete and that they should not expect any further communication about that particular order.   When you check the history in the version control tab in studio pro, you can see they have created the following two microflows IVK_SendOrderCompletedEmail and SUB_SendOrderCompletedEmail.   IVK_SendOrderCompletedEmail   SUB_SendOrderCompletedEmail   Provide a list of everything that according to you does not comply with the microflow best practices, and explain why it is incorrect. Make sure your Mendix account is public so we know who you are as our next potential first-time winner! The winner will be announced in the next release video.    Good luck everyone!
asked
7 answers
5

IVK_SendOrderCompletedEmail
 - Following general naming conventions this microflow should be named something closer to 'ACT_Orders_SendOrderCompletedEmail'
 - The first retrieve in this microflow should use square brackets for separation rather than 'and'. So something more like [Status = 'Delivered' or Status = 'Completed'][CompletedEmailSent = false()]
 - There shouldn't be a commit or refresh within the loop, this should be done after the loop has completed and should be done on the list rather than each individual object within that list
 - When calling the SUB_SendOrderCompletedEmail microflow you should pass the list of orders as a parameter rather than performing the same retrieve action again

SUB_SendOrderCompletedEmail
 - Again with the naming conventions, this microflow name should represent the main entity being worked on, so it should be something more like SUB_Email_SendOrderCompletedEmail
 - The retrieve should be replaced with an incoming parameter from the calling microflow to /slightly/ increase performance and reduce unnecessary redundancy
 - We should do a single retrieve here before the loop on all emails, and then perform a 'List Operation - Find' on the retrieved list of email objects to find a specific email through it's association with the IteratorOrder
 - Instead of committing the new email object within the loop it should be added to the retrieved list, and the retrieved list should be committed at the end of the microflow.
 - This is more a preference than best practices, but the branches of the decision should be uniform for readabilitiy's sake. In this case the false branch should be pointing down because that was the convention established in the calling microflow.

General Notes
 - Finally, again this is more of a preference than best practices, but it would be nice to include some kind of message to let the user know the process completed. Maybe even include how many emails were supposedly sent off in the message?
 - I appreciate the annotation. It's the sign of a empathetic developer, and that's awesome.

answered
3
  • "Retrieve list of Orders from database" is already set on Main microflow(IVK_SendOrderCompletedEmail), it shouldn't repeat again on Sub, as the object can be passed from IVK_SendOrderCompletedEmail.
  •  Sub (Sub_SendOrderCompletedEmail) should have a parameter from Main microflow(IVK_SendOrderCompletedEmail).
  • On Main microflow (IVK_SendOrderCompletedEmail), the Sub_SendOrderCompletedEmail call activity should park inside the looping activity.
  • On Main Microflow (IVK_SendOrderCompletedEmail), in the retrieve activity, the condition should have bracket [ ] between “and” instead.
  • On Sub_SendOrderComlpetedEmail, the looping is not required if the Main microflow(IVK_SendOrderCompletedEmail) already loop the data.
  • On Sub_SendOrderComlpetedEmail, should use Merge activity to merge and call the Sub_SendEmail microflow.
  • Suggested to add “audit trail” at the end of the activity, to keep track the activity incase there is an email sent failed.
  • Suggested to follow proper naming convention for microflow, such as {Prefix}_{Entity}_{Operation} -”ACT_”, (“IVK_” is used historically)
answered
2
  • The two retrieve have the same constraint, but it is not intuïtive if they will result in the same orderlist, since after the first and before the second retrieve the orders get their status set to ‘Completed’. The second retrieve is unnecessary though. Only one order should get passed;
  • Retrieval of the orders now happens in both microflows.  SUB_SendOrderCompletedEmail should be called inside instead of after the loop and only pass one Order;
  • There are several commits in loops
    • the ChangeIteratorOrders should not commit. After the loop, the OrdersList should get committed.
    • the commit of ‘NewEmail’ is likely unnecessary since it will not ever get read; If however, there is a need for it, then the commit should be done after the loop, by first adding it to a list of NewEmailList;
  • According to developer guidelines the microflow names:
    • SUB_SendOrderCompletedEmail and SUB_SendEmail should not be prefixed with ‘SUB_’ but not be prefixed at all;
    • IVK_ should be replaced with Act_ with only the first character as a capital letter;
    • SendEmail should be named Email_Send;
    • SUB_SendOrderCompletedEmail should be named Order_SendCompletedEmail, since you will be passing one order to the microflow;
    • IVK_SendOrderCompletedEmail should be named Act_Orders_SendCompletedEmail, Orders plural since for multiple orders you will be sending the ‘Completed’-mail;
  • The entity name ‘Orders’ is plural and has to get changed to singular. Afterward, also the variable name ‘OrdersList’ should be changed to ‘OrderList’. Association ‘Email_Orders’ will get renamed by Mendix when the entity gets renamed;
  • The connecting line from the annotation should not cross the activity; Better place this annotation beneath the activity;
  • The false-branch of the exclusive split in SUB_SendOrderCompletedEmail goes up. It is more intuïtive when it goes down;
  • The annotation in SUB_SendOrderCompletedEmail should be inside the loop;
  • The attribute CompletedEmailSent is a boolean, therefor a better name is CompletedEmailIsSent;
  • The retrieve- or create-email should be a submicroflow returning an Email-object which you pass to SUB_EmailSend.
  • The annotation in the first microflow implies nothing will happen with Orders on status Delivered. If that is the intention of this story then changing status should be removed. If however, it is part of the story, then the two functions ‘setting status to delivered’ and ‘send emails for each completed order’ should be separated into two microflows, ‘Orders_Delivered_SetToCompleted’ and ‘Orders_Completed_SendEmail’ and get called from a parent microflow, so no prefix.
answered
1

Ok, here are my two cents about these microflows:

First of all, I'm not commenting on the naming convention. This depends on whether the functionality should be performed automatically for the relevant orders, or be a manual function for a user to invoke on a selection of orders from, for example, a data grid. This is not specified.

Second of all, the most obvious, single action activity issues:

  • IVK_SendOrderCompletedEmail
    • Retrieve construction is not correct. Current construction only checks CompletedEmailSent = false() if Status is ‘Completed’. Brackets should be placed around the Status checks
    • Entity name is plural (Orders), should be singular (Order)
    • Change action in the loop should not be committed or refreshed. The commit should happen outside the loop
    • OrderList should be provided as a parameter to SUB_SendOrderCompletedEmail
  • SUB_SendOrderCompletedEmail
    • Annotation should not cross the action
    • Retrieve of list is unnecessary if list is provided as parameter
    • Details of retrieve action of Email could be put in caption of action instead of annotation (Retrieve Email associated to Order from DB)
    • False condition value could go down, but this would increase the size of the loop, because currently the Iterator-parameter fits nicely in the open corner. The most important thing is that the true condition value continues to the right.
    • Create action in the loop should not be committed or refreshed. The commit should happen outside the loop

These are all single action remarks. My peer review would go further (although it might not all have to do with best practices):

  • The loop in IVK_SendOrderCompletedEmail should not be in this flow. It changes the status, and this has no influence on an Email being sent or not. Changing the status is independent of sending an Email and also does not fall within the description of the microflow-name and should be a separate microflow.
  • If Emails are sent for both Completed and Delivered orders, and the status can be changed here without any other changes, what is the functional difference between the two statusses? If an order can change from Delivered to Completed, and the only change is that an Email is sent, but Emails are also sent for Completed orders, than there is no difference in the two statusses and they should be combined.
  • The 1-1 association between Email and Order seems very strange. For Emails I would expect that a new object is created every time when a new Email is sent. This would mean that the retrieve of the associated Email is not necessary and a new object can be created every time.
    • If this is required, than the functionality should still be in a submicroflow (GetOrCreate_… / FetchOrCreate_...) which would result in the elimination of one of the SUB_SendEmail microflow calls
answered
1

1. Create list widget once created app in Mendix.
2. Add Entity in Domain Model to get the list of all the order status
3. If Order Status is delivered then Call Microflow (IVK_SendOrderCompletedEmail) to send email and set CompletedEmailSent = false. 
4. If Order Status is completed then Call Microflow (IVK_SendOrderCompletedEmail) to send email and set CompletedEmailSent = true.
4. Create another Microflow (SUB_SendOrderCompletedEmail) and get the list of Order which completed and CompletedEmailSent = true
   then create email if email is an empty and call SUB_Sendemail to send email.

answered
1

For the IVK flow:

  • IVK is an old prefix and replaced by ACT
  • The names are not describing what's happening, because it's not for one order, but for all completed orders. And the entity should be in the microflow name (for both). A better name could be ACT_Order_SendEmailForDeliveredAndCompletedOrders.
  • The comments for the retrieve shows that Delivered and Completed orders will be retrieved, but the deciscion in the loop only checks delivered (according to the text)
  • The deciscion in the loop is unnecessary when the right status is checked in the retrieve action
  • The changed order is committed directly instead of commiting the orderlist, which will decrease performance. 
  • The microflow call for SUB_SendOrder should be in the loop to prevent retrieving the orderlist again and I prefer to call it before changing the status to the next step.
  • The whole loop should be in a seperate microflow, so you can add error handling at the 'top' level (which is the IVK microflow).

 

For the SUB flow:

  • Retrieving the orderlist again is silly, it's a better idea to call this microflow per order as suggested above. The name then could be SUB_Order_SendCompletedEmailForOrder. A flow per order also gives a better ability to re-use this flow. If you don't want to have it per order, then an order list parameter is also a improvement.
  • The loop name is IteratorOrders wich suggests multiple orders, but it's only one. $IteratorOrder is a better name. (Improvement for Mendix: if you use the create loop action from the suggestion menu, this variable is called $IteratorOrderList which makes no sense.)
  • Getting the e-mail should be in a seperate microflow. It's common functionality (get a e-mail linked to an order) and makes this flow cleaner because you only need SUB_SendEmail once.
  • There is no error handling when sending an e-mail fails.
  • It's possible to collect the mails in a list and commit them once, but when mails are send directly I guess it's necessary to commit them before sending.
     
answered
0
  • Entity name should not be in plural form . Should be updated to Order ← Orders .
  • Use of color can add a great visibility value to go through microlfows. Use of Color can be implemented. Ex:- could have used Yellow color for sub microflow and green color for commit activity.
  •  The  microflow naming convention should follow the pattern prefix_Entity_Operationperfomed , so use  Act_Order_SendOrderCompleteEmail  instead of IVK_SendOrderCompletedEmail and Sub_Email_SendOrderCompletedEmail instead of SUB_SendOrderCompletedEmail.
  •  In the IVK_SendOrderCompletedEmail the delievered status conditoon can be removed since email is being sent only to Compledted orders. Rebuild the xpath for the first retrieve action for order: [status=’Completed’] [CompletedEmailSent=false()] 
  • The IVK_SendOrderCompletedEmail  is meant to sent email for completed orders,so the change activity for order status can be handled in a separte microflow. 

 

Reconstruct the second microlfow by following below points :

  • Pass Order object from the main microflow , so that loop will be eliminated.
  •  Use yellow color for sub microlfow Sub_SendEmail.
  • Change the the status of the booleean value CompletedEmailSent to True/False  at the end of the second microflow based on the return value from the Sub_Send Email, sicne email is sent only  once according to the user story (Not implemented in the flow).
  • Association between Order and email object is not necessary. Retrieve of Email object with association is notneeded since the email is sent only once per completed order, new email object can be created every time.

                                                                   Reconstructing the second microflow eliminates Additional loop and Additional retrieve activity.

Reconstruct the first microflow by following below points:   

  • Delete the change activity for Order status, since this can be handled in other flow.
  •  Call the Submicroflow Sub_Email_SendOrderCompletedEmail  inside the loop and give yellow color.
  •  Annotation can be added above the Exclusive split, which is not necessary if we reconstruct the microlfow.

     

 

               

answered