[GitHub] [netbeans] sdedic commented on a diff in pull request #5543: When adding an Autonomous DB, present profiles instead of tenancies

["sdedic (via GitHub)" <gi...@apache.org>]

sdedic commented on code in PR #5543:
URL: https://github.com/apache/netbeans/pull/5543#discussion_r1116567440


##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/actions/AddADBAction.java:
##########
@@ -83,47 +89,82 @@ public class AddADBAction implements ActionListener {
     private static final String USERNAME = "username"; //NOI18N
     private static final String PASSWORD = "password"; //NOI18N
 
+    @NbBundle.Messages({
+        "MSG_CollectingProfiles=Searching for OCI Profiles",
+        "MSG_CollectingProfiles_Text=Loading OCI Profiles",
+        "MSG_CollectingItems=Loading OCI contents",
+        "MSG_CollectingItems_Text=Listing compartments and databases",
+    })
     @Override
     public void actionPerformed(ActionEvent e) {
         Map<String, Object> result = new HashMap<> ();
         
         NotifyDescriptor.ComposedInput ci = new NotifyDescriptor.ComposedInput(Bundle.AddADB(), 3, new Callback() {
-            Map<Integer, List> values = new HashMap<> ();
-            
+            Map<Integer, Map> values = new HashMap<> ();
+
             @Override
             public NotifyDescriptor createInput(NotifyDescriptor.ComposedInput input, int number) {
                 if (number == 1) {
-                    List<TenancyItem> tenancies = new ArrayList<>();
-                    for (OCIProfile p : OCIManager.getDefault().getConnectedProfiles()) {
-                        p.getTenancy().ifPresent(tenancies::add);
+                    ProgressHandle h = ProgressHandle.createHandle(Bundle.MSG_CollectingProfiles());
+                    h.start();
+                    h.progress(Bundle.MSG_CollectingProfiles_Text());
+        
+                    Map<OCIProfile, Tenancy> profiles = new LinkedHashMap<>();
+                    Map<String, TenancyItem> tenancyItems = new LinkedHashMap<>();
+                    try {
+                        for (OCIProfile p : OCIManager.getDefault().getConnectedProfiles()) {
+                            TenancyItem t = p.getTenancy().orElse(null);

Review Comment:
   Acutally, I don't think so. OCIProfile IMHO misuses `Optional`, where a simple `null` indicating that value is not available could be sufficient. I don't find any value in replacing `if` with `Optional` in a simple 
   imperative code - except that it looks fancy, and is harder to debug, and consumes more resources at runtime. 
   The `orElse` here is a way how to convert the unnecessary Optional into value that can be used. The other options would be an temporary var and `ifPresent` test (possible) and change of `getTenancy()` (which would require me to analyse **actuall** usages whether the Optional is generally useful or not for clients).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists