review the use of String and switch to &str when possible (#22)

* review the use of `String` and switch to `&str` when possible

* get rid of more String

chorus_book/src/guide-transport.md

@@ -100,9 +100,9 @@ We need to construct a `HttpTransportConfig` using the `HttpTransportConfigBuild
 {{#include ./header.txt}}
 # use chorus_lib::transport::http::{HttpTransport, HttpTransportConfigBuilder};
 // `Alice` listens on port 8080 on localhost
-let config = HttpTransportConfigBuilder::for_target(Alice, ("localhost".to_string(), 8080))
+let config = HttpTransportConfigBuilder::for_target(Alice, ("localhost", 8080))
                 // Connect to `Bob` on port 8081 on localhost
-                .with(Bob, ("localhost".to_string(), 8081))
+                .with(Bob, ("localhost", 8081))
                 .build();
 let transport = HttpTransport::new(config);
 ```
@@ -111,14 +111,14 @@ In the above example, the transport will start the HTTP server on port 8080 on l
 
 ## Creating a Custom Transport
 
-You can also create your own transport by implementing the `Transport` trait. It might be helpful to first build a `TransportConfig` to have the the information that you need for each `ChoreographyLocation`, and then have a constructor that takes the `TransportConfig` and builds the `Transport` based on it. While the syntax is similar to `HttpTransportConfig`, which is `HttpTransportConfigBuilder::for_target(target_location, target_information)`, chained with information about other locations using the `.with(other_location, other_location_information)`,  the type of information for each `ChoreographyLocation` might diverge from the `(host_name, port)` format presented in `HttpTransport`. In some cases, the `target_information` could even have a different type than the following `other_location_information` types. But all the `other_location_information`s should have the same type.
+You can also create your own transport by implementing the `Transport` trait. It might be helpful to first build a `TransportConfig` to have the the information that you need for each `ChoreographyLocation`, and then have a constructor that takes the `TransportConfig` and builds the `Transport` based on it. While the syntax is similar to `HttpTransportConfig`, which is `HttpTransportConfigBuilder::for_target(target_location, target_information)`, chained with information about other locations using the `.with(other_location, other_location_information)`, the type of information for each `ChoreographyLocation` might diverge from the `(host_name, port)` format presented in `HttpTransport`. In some cases, the `target_information` could even have a different type than the following `other_location_information` types. But all the `other_location_information`s should have the same type.
 
 ```rust
 {{#include ./header.txt}}
 # use chorus_lib::transport::TransportConfigBuilder;
 let config = TransportConfigBuilder::for_target(Alice, ())
-                .with(Bob, ("localhost".to_string(), 8081))
-                .with(Carol, ("localhost".to_string(), 8082))
+                .with(Bob, ("localhost", 8081))
+                .with(Carol, ("localhost", 8082))
                 .build();
 ```
 

chorus_lib/examples/tic-tac-toe.rs

@@ -296,14 +296,11 @@ fn main() {
         'X' => {
             let config = HttpTransportConfigBuilder::for_target(
                 PlayerX,
-                (args.hostname.as_str().to_string(), args.port),
+                (args.hostname.as_str(), args.port),
             )
             .with(
                 PlayerO,
-                (
-                    args.opponent_hostname.as_str().to_string(),
-                    args.opponent_port,
-                ),
+                (args.opponent_hostname.as_str(), args.opponent_port),
             )
             .build();
 
@@ -317,14 +314,11 @@ fn main() {
         'O' => {
             let config = HttpTransportConfigBuilder::for_target(
                 PlayerO,
-                (args.hostname.as_str().to_string(), args.port),
+                (args.hostname.as_str(), args.port),
             )
             .with(
                 PlayerX,
-                (
-                    args.opponent_hostname.as_str().to_string(),
-                    args.opponent_port,
-                ),
+                (args.opponent_hostname.as_str(), args.opponent_port),
             )
             .build();
 

chorus_lib/src/core.rs

@@ -317,7 +317,7 @@ pub trait Choreography<R = ()> {
 /// The type parameter `TargetLocation` is the target `ChoreographyLocation`.
 pub trait Transport<L: LocationSet, TargetLocation: ChoreographyLocation> {
     /// Returns a list of locations.
-    fn locations(&self) -> Vec<String>;
+    fn locations(&self) -> Vec<&'static str>;
     /// Sends a message from `from` to `to`.
     fn send<V: Portable>(&self, from: &str, to: &str, data: &V) -> ();
     /// Receives a message from `from` to `at`.
@@ -396,7 +396,7 @@ where
         > {
             target: PhantomData<L1>,
             transport: &'a B,
-            locations: Vec<String>,
+            locations: Vec<&'static str>,
             marker: PhantomData<L>,
             projector_location_set: PhantomData<LS>,
         }
@@ -478,15 +478,14 @@ where
                 &self,
                 choreo: C,
             ) -> R {
-                let locs_vec =
-                    Vec::from_iter(S::to_string_list().into_iter().map(|s| s.to_string()));
+                let locs_vec = S::to_string_list();
 
                 for location in &locs_vec {
                     if *location == T::name().to_string() {
                         let op = EppOp {
                             target: PhantomData::<T>,
                             transport: self.transport,
-                            locations: locs_vec.clone(),
+                            locations: locs_vec,
                             marker: PhantomData::<S>,
                             projector_location_set: PhantomData::<LS>,
                         };

chorus_lib/src/transport.rs

@@ -9,13 +9,14 @@ use std::marker::PhantomData;
 
 /// A generic struct for configuration of `Transport`.
 #[derive(Clone)]
-pub struct TransportConfig<Target: ChoreographyLocation, TargetInfo, L: LocationSet, Info> {
+pub struct TransportConfig<'a, Target: ChoreographyLocation, TargetInfo, L: LocationSet, Info> {
     /// The information about locations
-    pub info: HashMap<String, Info>,
+    pub info: HashMap<&'static str, Info>,
     /// The information about the target choreography
     pub target_info: (Target, TargetInfo),
     /// The struct is parametrized by the location set (`L`).
     location_set: PhantomData<L>,
+    lifetime: PhantomData<&'a ()>,
 }
 
 /// A builder for `TransportConfig`.
@@ -38,52 +39,62 @@ pub struct TransportConfig<Target: ChoreographyLocation, TargetInfo, L: Location
 ///    .with(Bob, "value_for_bob".to_string())
 ///    .build();
 /// ```
-pub struct TransportConfigBuilder<Target: ChoreographyLocation, TargetInfo, L: LocationSet, Info> {
+pub struct TransportConfigBuilder<
+    'a,
+    Target: ChoreographyLocation,
+    TargetInfo,
+    L: LocationSet,
+    Info,
+> {
     target: (Target, TargetInfo),
     location_set: PhantomData<L>,
-    info: HashMap<String, Info>,
+    info: HashMap<&'static str, Info>,
+    lifetime: PhantomData<&'a ()>,
 }
 
-impl<Target: ChoreographyLocation, TargetInfo, Info>
-    TransportConfigBuilder<Target, TargetInfo, LocationSet!(Target), Info>
+impl<'a, Target: ChoreographyLocation, TargetInfo, Info>
+    TransportConfigBuilder<'a, Target, TargetInfo, LocationSet!(Target), Info>
 {
     /// Creates a new `TransportConfigBuilder` instance for a given target.
     pub fn for_target(target: Target, info: TargetInfo) -> Self {
         Self {
             target: (target, info),
             location_set: PhantomData,
             info: HashMap::new(),
+            lifetime: PhantomData,
         }
     }
 }
 
-impl<Target: ChoreographyLocation, TargetInfo, L: LocationSet, Info>
-    TransportConfigBuilder<Target, TargetInfo, L, Info>
+impl<'a, Target: ChoreographyLocation, TargetInfo, L: LocationSet, Info>
+    TransportConfigBuilder<'a, Target, TargetInfo, L, Info>
 {
     /// Adds information about a new `ChoreographyLocation`.
     ///
     /// This method tells the builder that the choreography involves a new location and how to communicate with it.
-    pub fn with<NewLocation: ChoreographyLocation>(
+    pub fn with<'b, NewLocation: ChoreographyLocation>(
         self,
         location: NewLocation,
         info: Info,
-    ) -> TransportConfigBuilder<Target, TargetInfo, HCons<NewLocation, L>, Info> {
+    ) -> TransportConfigBuilder<'b, Target, TargetInfo, HCons<NewLocation, L>, Info> {
         _ = location;
         let mut new_info = self.info;
-        new_info.insert(NewLocation::name().to_string(), info);
+        new_info.insert(NewLocation::name(), info);
         TransportConfigBuilder {
             target: self.target,
             location_set: PhantomData,
             info: new_info,
+            lifetime: PhantomData,
         }
     }
 
     /// Builds a `TransportConfig` instance.
-    pub fn build(self) -> TransportConfig<Target, TargetInfo, L, Info> {
+    pub fn build<'b>(self) -> TransportConfig<'b, Target, TargetInfo, L, Info> {
         TransportConfig {
             info: self.info,
             target_info: self.target,
             location_set: PhantomData,
+            lifetime: PhantomData,
         }
     }
 }

chorus_lib/src/transport/http.rs

@@ -18,10 +18,11 @@ use crate::{
     utils::queue::BlockingQueue,
 };
 
-type QueueMap = HashMap<String, BlockingQueue<String>>;
+type QueueMap = HashMap<&'static str, BlockingQueue<String>>;
 
 /// Config for `HttpTransport`.
-pub type HttpTransportConfig<L, Target> = TransportConfig<Target, (String, u16), L, (String, u16)>;
+pub type HttpTransportConfig<'a, L, Target> =
+    TransportConfig<'a, Target, (&'a str, u16), L, (&'a str, u16)>;
 
 /// A builder for `HttpTransportConfig`.
 ///
@@ -37,19 +38,19 @@ pub type HttpTransportConfig<L, Target> = TransportConfig<Target, (String, u16),
 /// # #[derive(ChoreographyLocation)]
 /// # struct Bob;
 /// #
-/// let transport_config = HttpTransportConfigBuilder::for_target(Alice, ("0.0.0.0".to_string(), 9010))
-///   .with(Bob, ("example.com".to_string(), 80))
+/// let transport_config = HttpTransportConfigBuilder::for_target(Alice, ("0.0.0.0", 9010))
+///   .with(Bob, ("example.com", 80))
 ///   .build();
 /// ```
-pub type HttpTransportConfigBuilder<Target, L> =
-    TransportConfigBuilder<Target, (String, u16), L, (String, u16)>;
+pub type HttpTransportConfigBuilder<'a, Target, L> =
+    TransportConfigBuilder<'a, Target, (&'a str, u16), L, (&'a str, u16)>;
 
 /// The header name for the source location.
 const HEADER_SRC: &str = "X-CHORUS-SOURCE";
 
 /// The HTTP transport.
-pub struct HttpTransport<L: LocationSet, TLocation> {
-    config: HashMap<String, (String, u16)>,
+pub struct HttpTransport<'a, L: LocationSet, TLocation> {
+    config: HashMap<&'static str, (&'a str, u16)>,
     agent: Agent,
     server: Arc<Server>,
     join_handle: Option<thread::JoinHandle<()>>,
@@ -58,16 +59,16 @@ pub struct HttpTransport<L: LocationSet, TLocation> {
     target_location: PhantomData<TLocation>,
 }
 
-impl<L: LocationSet, TLocation: ChoreographyLocation> HttpTransport<L, TLocation> {
+impl<'a, L: LocationSet, TLocation: ChoreographyLocation> HttpTransport<'a, L, TLocation> {
     /// Creates a new `HttpTransport` instance from the configuration.
-    pub fn new<Index>(http_config: HttpTransportConfig<L, TLocation>) -> Self
+    pub fn new<Index>(http_config: HttpTransportConfig<'a, L, TLocation>) -> Self
     where
         TLocation: Member<L, Index>,
     {
         let queue_map: Arc<QueueMap> = {
             let mut m = HashMap::new();
             for loc in L::to_string_list() {
-                m.insert(loc.to_string(), BlockingQueue::new());
+                m.insert(loc, BlockingQueue::new());
             }
             Arc::new(m.into())
         };
@@ -119,18 +120,18 @@ impl<L: LocationSet, TLocation: ChoreographyLocation> HttpTransport<L, TLocation
     }
 }
 
-impl<L: LocationSet, TLocation> Drop for HttpTransport<L, TLocation> {
+impl<'a, L: LocationSet, TLocation> Drop for HttpTransport<'a, L, TLocation> {
     fn drop(&mut self) {
         self.server.unblock();
         self.join_handle.take().map(thread::JoinHandle::join);
     }
 }
 
-impl<L: LocationSet, TLocation: ChoreographyLocation> Transport<L, TLocation>
-    for HttpTransport<L, TLocation>
+impl<'a, L: LocationSet, TLocation: ChoreographyLocation> Transport<L, TLocation>
+    for HttpTransport<'a, L, TLocation>
 {
-    fn locations(&self) -> Vec<String> {
-        Vec::from_iter(self.config.keys().map(|s| s.clone()))
+    fn locations(&self) -> Vec<&'static str> {
+        self.config.keys().cloned().collect()
     }
 
     fn send<V: Portable>(&self, from: &str, to: &str, data: &V) -> () {
@@ -176,10 +177,9 @@ mod tests {
 
         let mut handles = Vec::new();
         {
-            let config =
-                HttpTransportConfigBuilder::for_target(Alice, ("0.0.0.0".to_string(), 9010))
-                    .with(Bob, ("localhost".to_string(), 9011))
-                    .build();
+            let config = HttpTransportConfigBuilder::for_target(Alice, ("0.0.0.0", 9010))
+                .with(Bob, ("localhost", 9011))
+                .build();
 
             handles.push(thread::spawn(move || {
                 wait.recv().unwrap(); // wait for Bob to start
@@ -188,8 +188,8 @@ mod tests {
             }));
         }
         {
-            let config = HttpTransportConfigBuilder::for_target(Bob, ("0.0.0.0".to_string(), 9011))
-                .with(Alice, ("localhost".to_string(), 9010))
+            let config = HttpTransportConfigBuilder::for_target(Bob, ("0.0.0.0", 9011))
+                .with(Alice, ("localhost", 9010))
                 .build();
 
             handles.push(thread::spawn(move || {
@@ -211,10 +211,9 @@ mod tests {
 
         let mut handles = Vec::new();
         {
-            let config =
-                HttpTransportConfigBuilder::for_target(Alice, ("0.0.0.0".to_string(), 9020))
-                    .with(Bob, ("localhost".to_string(), 9021))
-                    .build();
+            let config = HttpTransportConfigBuilder::for_target(Alice, ("0.0.0.0", 9020))
+                .with(Bob, ("localhost", 9021))
+                .build();
 
             handles.push(thread::spawn(move || {
                 signal.send(()).unwrap();
@@ -223,8 +222,8 @@ mod tests {
             }));
         }
         {
-            let config = HttpTransportConfigBuilder::for_target(Bob, ("0.0.0.0".to_string(), 9021))
-                .with(Alice, ("localhost".to_string(), 9020))
+            let config = HttpTransportConfigBuilder::for_target(Bob, ("0.0.0.0", 9021))
+                .with(Alice, ("localhost", 9020))
                 .build();
 
             handles.push(thread::spawn(move || {

chorus_lib/src/transport/local.rs

@@ -124,7 +124,7 @@ impl<L: LocationSet> LocalTransportChannelBuilder<L> {
 ///
 /// All locations must share the same `LocalTransportChannel` instance. `LocalTransportChannel` implements `Clone` so that it can be shared across threads.
 pub struct LocalTransport<L: LocationSet, TargetLocation> {
-    internal_locations: Vec<String>,
+    internal_locations: Vec<&'static str>,
     location_set: PhantomData<L>,
     local_channel: LocalTransportChannel<L>,
     target_location: PhantomData<TargetLocation>,
@@ -135,15 +135,8 @@ impl<L: LocationSet, TargetLocation> LocalTransport<L, TargetLocation> {
     pub fn new(target: TargetLocation, local_channel: LocalTransportChannel<L>) -> Self {
         _ = target;
 
-        let locations_list = L::to_string_list();
-
-        let mut locations_vec = Vec::new();
-        for loc in locations_list.clone() {
-            locations_vec.push(loc.to_string());
-        }
-
         LocalTransport {
-            internal_locations: locations_vec,
+            internal_locations: L::to_string_list(),
             location_set: PhantomData,
             local_channel,
             target_location: PhantomData,
@@ -154,7 +147,7 @@ impl<L: LocationSet, TargetLocation> LocalTransport<L, TargetLocation> {
 impl<L: LocationSet, TargetLocation: ChoreographyLocation> Transport<L, TargetLocation>
     for LocalTransport<L, TargetLocation>
 {
-    fn locations(&self) -> Vec<String> {
+    fn locations(&self) -> Vec<&'static str> {
         return self.internal_locations.clone();
     }