5

I am writing a client for a TCP server

use std::net::TcpStream;
use std::io::{Read, Write, Error};

pub struct Client {
    addr: String,
    conn: Option<TcpStream>,
}

impl Client {
    pub fn connect(&mut self) {
        let res = TcpStream::connect(&self.addr);
        match res {
            Ok(c) => {
                self.conn = Some(c);
            },
            Err(_) => panic!(),
        }
    }

    pub fn version(self) -> Result<[u8; 8], Error> {
        let mut ver: [u8; 8] = [0;8];
        let command_string = b"VERSION\r\n";
        let res = self.conn.unwrap().write(&command_string[0..]);
        match res {
            Ok(_) => self.conn.unwrap().read(&mut ver[..]),
            Err(e) => panic!(e)
        };
        Ok(ver)
    }
}

The version method returns an error

error[E0382]: use of moved value: `self.conn`

when I try to reuse the reference. I thought about using #[derive(Copy, Clone)] but since these traits are not implemented for TCPStream I am not sure what to do. What would be the best way to achieve reading and writing to a server in a method (btw there will be many such methods like version which will both read and write to the server)

I am still new to rust and trying to wrap my head around ownership concepts so I apologise if this is a stupid question but since I want to discuss various approaches (if indeed there are multiple approaches to solving this problem) to reuse un-Clonable/Copyable fields I thought this question was worth asking.

trent
  • 25,033
  • 7
  • 51
  • 90
Palash Nigam
  • 1,653
  • 3
  • 14
  • 26
  • 1
    I made a few, (to me) fairly conservative assumptions about your code to get it to compile; please review the changes and fix the code if my changes do not reflect your problem. – trent Sep 19 '19 at 20:27
  • thanks for adding the missing imports @trentcl it slipped my mind while drafting the question. – Palash Nigam Sep 19 '19 at 20:32

1 Answers1

4

The version method returns an error when I try to reuse the reference

self.conn is not a reference; it is an owned value. Because Option::unwrap takes it by value (and its type does not implement Copy), you can't reuse it after calling .unwrap() on it; that would lead to unsafety.

But you could just store the result of calling unwrap into a new variable, and use that instead:

    pub fn version(self) -> io::Result<[u8; 8]> {
        let mut ver: [u8; 8] = [0;8];
        const COMMAND_STRING: &[u8] = b"VERSION\r\n";
        let mut conn = self.conn.unwrap();
        conn.write(COMMAND_STRING)?;
        conn.read(&mut ver[..])?;
        Ok(ver)
    }

This is possible because io::Write::write and io::Read::read both take &mut self, which means that neither one needs to take conn by value. Instead, conn will be dropped at the end of version. See also How to prevent a value from being moved?

I've made several further changes, which you may consider suggestions:

  • io::Result<_> is an alias for Result<_, io::Error>, so I changed the declared return type. This is a common pattern in many libraries, and I find this form easier to understand quickly.
  • conn.read() returns an io::Result, which was previously being ignored. Now conn.write() and conn.read() both raise errors to the caller with the special ? operator. You can replace ? with .unwrap() if you want errors to panic instead of propagate. See What is this question mark operator about?
  • COMMAND_STRING is a constant. It's debatable whether having this name at all is useful compared to just conn.write(b"VERSION\r\n"), but if you're going to have it, a const seems the right way to go. In principle the compiler can optimize consts better than regular variables; in practice it's not likely to make much difference in this case, but when a const is more explicit about how the value is used, there's no reason not to.

Using an actual reference

If you actually want version to take self by reference, and therefore you don't want it to unwrap and destroy self.conn, you can use Option::as_ref to make that work.

    pub fn version(&self) -> io::Result<[u8; 8]> {  // NOTE: `&self`
        let mut ver: [u8; 8] = [0;8];
        let mut conn = self.conn.as_ref().unwrap(); // NOTE: `.as_ref()`
        conn.write(b"VERSION\r\n")?;
        conn.read(&mut ver[..])?;
        Ok(ver)
    }

Playground link

as_ref turns a &Option<_> into a Option<&_>, so that unwrapping it does not consume self.conn but just returns the reference. See Cannot move out of borrowed content when unwrapping.

trent
  • 25,033
  • 7
  • 51
  • 90