Store pointer to storage directly

document and rename internal function

wpilibc/src/main/native/cpp/Alert.cpp

@@ -26,12 +26,14 @@ Alert::Alert(std::string_view text, AlertType type)
     : Alert("Alerts", text, type) {}
 
 Alert::Alert(std::string_view group, std::string_view text, AlertType type)
-    : m_type(type), m_text(text), m_group{&GetGroupSendable(group)} {}
+    : m_type(type),
+      m_text(text),
+      m_activeAlerts{&GetGroupSendable(group).GetActiveAlertsStorage(m_type)} {}
 
 Alert::Alert(Alert&& other)
     : m_type{other.m_type},
       m_text{std::move(other.m_text)},
-      m_group{std::exchange(other.m_group, nullptr)},
+      m_activeAlerts{std::exchange(other.m_activeAlerts, nullptr)},
       m_active{std::exchange(other.m_active, false)},
       m_activeStartTime{other.m_activeStartTime} {}
 
@@ -42,7 +44,7 @@ Alert& Alert::operator=(Alert&& other) {
     // Now, swap moved-from state with other state
     std::swap(m_type, other.m_type);
     std::swap(m_text, other.m_text);
-    std::swap(m_group, other.m_group);
+    std::swap(m_activeAlerts, other.m_activeAlerts);
     std::swap(m_active, other.m_active);
     std::swap(m_activeStartTime, other.m_activeStartTime);
   }
@@ -60,9 +62,9 @@ void Alert::Set(bool active) {
 
   if (active) {
     m_activeStartTime = frc::RobotController::GetFPGATime();
-    m_group->GetSetForType(m_type).emplace(m_activeStartTime, m_text);
+    m_activeAlerts->emplace(m_activeStartTime, m_text);
   } else {
-    m_group->GetSetForType(m_type).erase({m_activeStartTime, m_text});
+    m_activeAlerts->erase({m_activeStartTime, m_text});
   }
   m_active = active;
 }
@@ -80,10 +82,9 @@ void Alert::SetText(std::string_view text) {
   m_text = text;
 
   if (m_active) {
-    auto& set = m_group->GetSetForType(m_type);
-    auto iter = set.find({m_activeStartTime, oldText});
-    auto hint = set.erase(iter);
-    set.emplace_hint(hint, m_activeStartTime, m_text);
+    auto iter = m_activeAlerts->find({m_activeStartTime, oldText});
+    auto hint = m_activeAlerts->erase(iter);
+    m_activeAlerts->emplace_hint(hint, m_activeStartTime, m_text);
   }
 }
 
@@ -106,14 +107,14 @@ void Alert::SendableAlerts::InitSendable(nt::NTSendableBuilder& builder) {
       "infos", [this]() { return GetStrings(AlertType::kInfo); }, nullptr);
 }
 
-std::set<Alert::PublishedAlert>& Alert::SendableAlerts::GetSetForType(
+std::set<Alert::PublishedAlert>& Alert::SendableAlerts::GetActiveAlertsStorage(
     AlertType type) {
   return const_cast<std::set<Alert::PublishedAlert>&>(
-      std::as_const(*this).GetSetForType(type));
+      std::as_const(*this).GetActiveAlertsStorage(type));
 }
 
-const std::set<Alert::PublishedAlert>& Alert::SendableAlerts::GetSetForType(
-    AlertType type) const {
+const std::set<Alert::PublishedAlert>&
+Alert::SendableAlerts::GetActiveAlertsStorage(AlertType type) const {
   switch (type) {
     case AlertType::kInfo:
     case AlertType::kWarning:
@@ -127,7 +128,7 @@ const std::set<Alert::PublishedAlert>& Alert::SendableAlerts::GetSetForType(
 
 std::vector<std::string> Alert::SendableAlerts::GetStrings(
     AlertType type) const {
-  auto set = GetSetForType(type);
+  auto set = GetActiveAlertsStorage(type);
   std::vector<std::string> output;
   output.reserve(set.size());
   for (auto& alert : set) {

wpilibc/src/main/native/include/frc/Alert.h

@@ -140,8 +140,14 @@ class Alert {
     SendableAlerts();
     void InitSendable(nt::NTSendableBuilder& builder) override;
 
-    std::set<PublishedAlert>& GetSetForType(AlertType type);
-    const std::set<PublishedAlert>& GetSetForType(AlertType type) const;
+    /**
+     * Returns a reference to the set of active alerts for the given type
+     * @param type the type
+     * @return reference to the set of active alerts for the type
+     */
+    std::set<PublishedAlert>& GetActiveAlertsStorage(AlertType type);
+    const std::set<PublishedAlert>& GetActiveAlertsStorage(
+        AlertType type) const;
 
    private:
     std::vector<std::string> GetStrings(AlertType type) const;
@@ -150,7 +156,7 @@ class Alert {
 
   AlertType m_type;
   std::string m_text;
-  SendableAlerts* m_group;  // could replace with pointer to type set
+  std::set<PublishedAlert>* m_activeAlerts;
   bool m_active = false;
   uint64_t m_activeStartTime;
 

wpilibj/src/main/java/edu/wpi/first/wpilibj/Alert.java

@@ -75,7 +75,7 @@ public enum AlertType {
   private boolean m_active;
   private long m_activeStartTime;
   private String m_text;
-  private SendableAlerts m_group;
+  private Set<PublishedAlert> m_activeAlerts;
 
   /**
    * Creates a new alert in the default group - "Alerts". If this is the first to be instantiated,
@@ -100,7 +100,7 @@ public Alert(String text, AlertType type) {
   public Alert(String group, String text, AlertType type) {
     m_type = type;
     m_text = text;
-    m_group = getGroupSendable(group);
+    m_activeAlerts = getGroupSendable(group).getActiveAlertsStorage(type);
   }
 
   /**
@@ -116,9 +116,9 @@ public void set(boolean active) {
 
     if (active) {
       m_activeStartTime = RobotController.getFPGATime();
-      m_group.getSetForType(m_type).add(new PublishedAlert(m_activeStartTime, m_text));
+      m_activeAlerts.add(new PublishedAlert(m_activeStartTime, m_text));
     } else {
-      m_group.getSetForType(m_type).remove(new PublishedAlert(m_activeStartTime, m_text));
+      m_activeAlerts.remove(new PublishedAlert(m_activeStartTime, m_text));
     }
     m_active = active;
   }
@@ -145,10 +145,8 @@ public void setText(String text) {
     var oldText = m_text;
     m_text = text;
     if (m_active) {
-      var set = m_group.getSetForType(m_type);
-      set.remove(
-          new PublishedAlert(m_activeStartTime, oldText));
-      set.add(new PublishedAlert(m_activeStartTime, m_text));
+      m_activeAlerts.remove(new PublishedAlert(m_activeStartTime, oldText));
+      m_activeAlerts.add(new PublishedAlert(m_activeStartTime, m_text));
     }
   }
 
@@ -174,15 +172,20 @@ public int compareTo(PublishedAlert o) {
 
   private static final class SendableAlerts implements Sendable {
     // TODO: I think we could use WeakReference here to automatically remove dangling alerts
-
     private final Map<AlertType, Set<PublishedAlert>> m_alerts = new HashMap<>();
 
-    public Set<PublishedAlert> getSetForType(AlertType type) {
+    /**
+     * Returns a reference to the set of active alerts for the given type
+     *
+     * @param type the type
+     * @return reference to the set of active alerts for the type
+     */
+    public Set<PublishedAlert> getActiveAlertsStorage(AlertType type) {
       return m_alerts.computeIfAbsent(type, _type -> new TreeSet<>());
     }
 
     private String[] getStrings(AlertType type) {
-      return getSetForType(type).stream().map(a -> a.text()).toArray(String[]::new);
+      return getActiveAlertsStorage(type).stream().map(a -> a.text()).toArray(String[]::new);
     }
 
     @Override

wpilibj/src/test/java/edu/wpi/first/wpilibj/AlertTest.java

@@ -1,3 +1,7 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
 package edu.wpi.first.wpilibj;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;