patternrustMinor
Replace string in file
Viewed 0 times
filestringreplace
Problem
This is my first module I wrote in Rust. I'm new to the language and couldn't find a way to easily replace some words in a file, so that's my go at it. If there's already something like that and I just missed it, feel free to point me to it.
I'm looking for advice as to how I can make this more efficient and more "rustician" "Rustacean", so I can publish a crate with this for others to use.
Since I have to call
main.rs
str_replace.rs
```
use std::path::Path;
use std::fs::File;
use std::io::prelude::{Read, Write};
use std::ops::Deref;
#[derive(Debug)]
pub struct StrReplace
{
/// String or &str?
data: String,
}
impl StrReplace
{
///
/// Creates StrReplace from the contents of a file at the given path
///
/// Tried to wrap the "try!" macro around "file.read_to_string",
/// like in the documentation, but I got an error:
///
/// error[E0308]: mismatched types
/// --> src/str_replace.rs:37:9
/// |
I'm looking for advice as to how I can make this more efficient and more "rustician" "Rustacean", so I can publish a crate with this for others to use.
Since I have to call
"string".to_string() all the time, &str would be easier for this case here I guess, but would it be better? When should I generally use String and when &str?main.rs
mod str_replace;
use str_replace::StrReplace;
fn main()
{
let mut str_test = StrReplace::from_str("hello 1 2 1 2 hello bye");
str_test.replace("hello".to_string(), "bye".to_string())
.replace("1".to_string(), "0".to_string())
.replace("2".to_string(), "1".to_string());
println!("{:?}", *str_test);
let mut file_test = StrReplace::from_file("src/test.txt");
file_test.replace("hello".to_string(), "bye".to_string())
.replace("1".to_string(), "0".to_string())
.replace("2".to_string(), "1".to_string())
.replace("\n".to_string(), " ".to_string());
println!("{:?}", file_test.to_str());
file_test.to_file("src/to_file_test.txt");
let replace_here_test = StrReplace::replace_here("hello".to_string(), "bye".to_string(), "hello hello".to_string());
println!("{:?}", replace_here_test);
}str_replace.rs
```
use std::path::Path;
use std::fs::File;
use std::io::prelude::{Read, Write};
use std::ops::Deref;
#[derive(Debug)]
pub struct StrReplace
{
/// String or &str?
data: String,
}
impl StrReplace
{
///
/// Creates StrReplace from the contents of a file at the given path
///
/// Tried to wrap the "try!" macro around "file.read_to_string",
/// like in the documentation, but I got an error:
///
/// error[E0308]: mismatched types
/// --> src/str_replace.rs:37:9
/// |
Solution
-
You should become familiar with rustfmt, which will help you enforce the community's coding style. For example, opening braces generally go on the same line (for functions, structs, impls, etc.):
-
You should write real tests, not just a
-
"std String.replace function" - Functions are referred to using the double colon syntax:
-
Speaking of that, don't explain the implementation details in documentation comments. Users of your code are the wrong audience for that.
-
Additionally, don't put "notes to oneself" as documentation comments. Regular comments are more appropriate for programmer-to-programmer communication.
-
There are typos in the doc comments (
-
None of the arguments need to be
-
-
Read Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument? to see why you should prefer
-
Storing a
-
"Does it make sense to implement this?": No, you should not implement
-
For
-
Chaining involves making a decision and tradeoffs;
str_replace.rs
main.rs
Now we get to the tough(er) love part of the review
I'm looking for advice [...] so I can publish a crate with this for others to use.
I don't want to discourage you or anyone else from publish
You should become familiar with rustfmt, which will help you enforce the community's coding style. For example, opening braces generally go on the same line (for functions, structs, impls, etc.):
-fn main()
-{
+fn main() {
-
You should write real tests, not just a
main method that exercises the code. That's more of an example.-
"std String.replace function" - Functions are referred to using the double colon syntax:
String::replace.-
Speaking of that, don't explain the implementation details in documentation comments. Users of your code are the wrong audience for that.
-
Additionally, don't put "notes to oneself" as documentation comments. Regular comments are more appropriate for programmer-to-programmer communication.
-
There are typos in the doc comments (
occurence). Some editors can be configured to automatically spell check comments, that might be worth investigating.-
None of the arguments need to be
Strings, which generally means that they should be &strs. Changing to &str reduces mandatory allocations (note the reduction of to_string) and also removes the need for &* (which probably could have just been & in the first place).-
File::open and File::create take anything that can be converted to a Path, and offering only a &str limits the functionality of those methods. It's important to understand why a file path is not a string.-
Read Why is it discouraged to accept a reference to a String (&String) or Vec (&Vec) as a function argument? to see why you should prefer
&str over &String in the overwhelming majority of cases.-
Storing a
String requires that there is always one allocation; replacing a constant string just got more inefficient. Perhaps a Cow would be useful.-
"Does it make sense to implement this?": No, you should not implement
Deref because you are not creating a smart pointer. Do you really want people to be able to call file_test.capacity() or str_test.len()? What would those even mean?-
For
try! to work, you must return a Result. Re-read The Rust Programming Language chapter on error handling for an in-depth analysis.-
Chaining involves making a decision and tradeoffs;
&mut self chaining generally requires an extra line to establish a variable, self chaining requires ownership and potentially re-assignment of the variable. It comes down to expected use cases.str_replace.rs
use std::path::Path;
use std::fs::File;
use std::io::prelude::{Read, Write};
#[derive(Debug)]
pub struct StrReplace {
data: String,
}
impl StrReplace {
/// Creates StrReplace from the contents of a file at the given path
pub fn from_file(path: &str) -> StrReplace {
let filepath = Path::new(path);
let mut file = File::open(filepath).unwrap();
let mut data = String::new();
file.read_to_string(&mut data).expect("Failed to read file.");
StrReplace { data: data }
}
/// Creates StrReplace from a given &str
pub fn from_str(str: &str) -> StrReplace {
StrReplace { data: str.to_string() }
}
/// Replace the occurence of one string with another
/// returns self for chainability.
pub fn replace(&mut self, search: &str, replacement: &str) -> &mut Self {
self.data = self.data.replace(search, replacement);
self
}
/// Writes the possibly mutated data to a file at the given destination
pub fn to_file(&self, dst: &str) {
let mut file = File::create(dst).unwrap();
file.write_all(self.data.as_bytes()).expect("Failed to write file.");
}
/// Makes a &str out of StrReplace for further use
pub fn to_str(&self) -> &str {
&self.data
}
/// Replace the occurence of one string with another
/// without creating a StrReplace
pub fn replace_here(search: &str, replacement: &str, input: &str) -> String {
input.replace(search, replacement)
}
}main.rs
mod str_replace;
use str_replace::StrReplace;
fn main() {
let mut str_test = StrReplace::from_str("hello 1 2 1 2 hello bye");
str_test.replace("hello", "bye")
.replace("1", "0")
.replace("2", "1");
println!("{:?}", str_test);
let str_test = "hello 1 2 1 2 hello bye"
.replace("hello", "bye")
.replace("1", "0")
.replace("2", "1");
println!("{:?}", str_test);
let mut file_test = StrReplace::from_file("src/test.txt");
file_test.replace("hello", "bye")
.replace("1", "0")
.replace("2", "1")
.replace("\n", " ");
println!("{:?}", file_test.to_str());
file_test.to_file("src/to_file_test.txt");
let replace_here_test = StrReplace::replace_here("hello", "bye", "hello hello");
println!("{:?}", replace_here_test);
}Now we get to the tough(er) love part of the review
I'm looking for advice [...] so I can publish a crate with this for others to use.
I don't want to discourage you or anyone else from publish
Code Snippets
use std::path::Path;
use std::fs::File;
use std::io::prelude::{Read, Write};
#[derive(Debug)]
pub struct StrReplace {
data: String,
}
impl StrReplace {
/// Creates StrReplace from the contents of a file at the given path
pub fn from_file(path: &str) -> StrReplace {
let filepath = Path::new(path);
let mut file = File::open(filepath).unwrap();
let mut data = String::new();
file.read_to_string(&mut data).expect("Failed to read file.");
StrReplace { data: data }
}
/// Creates StrReplace from a given &str
pub fn from_str(str: &str) -> StrReplace {
StrReplace { data: str.to_string() }
}
/// Replace the occurence of one string with another
/// returns self for chainability.
pub fn replace(&mut self, search: &str, replacement: &str) -> &mut Self {
self.data = self.data.replace(search, replacement);
self
}
/// Writes the possibly mutated data to a file at the given destination
pub fn to_file(&self, dst: &str) {
let mut file = File::create(dst).unwrap();
file.write_all(self.data.as_bytes()).expect("Failed to write file.");
}
/// Makes a &str out of StrReplace for further use
pub fn to_str(&self) -> &str {
&self.data
}
/// Replace the occurence of one string with another
/// without creating a StrReplace
pub fn replace_here(search: &str, replacement: &str, input: &str) -> String {
input.replace(search, replacement)
}
}mod str_replace;
use str_replace::StrReplace;
fn main() {
let mut str_test = StrReplace::from_str("hello 1 2 1 2 hello bye");
str_test.replace("hello", "bye")
.replace("1", "0")
.replace("2", "1");
println!("{:?}", str_test);
let str_test = "hello 1 2 1 2 hello bye"
.replace("hello", "bye")
.replace("1", "0")
.replace("2", "1");
println!("{:?}", str_test);
let mut file_test = StrReplace::from_file("src/test.txt");
file_test.replace("hello", "bye")
.replace("1", "0")
.replace("2", "1")
.replace("\n", " ");
println!("{:?}", file_test.to_str());
file_test.to_file("src/to_file_test.txt");
let replace_here_test = StrReplace::replace_here("hello", "bye", "hello hello");
println!("{:?}", replace_here_test);
}let mut str_test = StrReplace::from_str("hello 1 2 1 2 hello bye");
str_test.replace("hello", "bye")
.replace("1", "0")
.replace("2", "1");
println!("{:?}", str_test);let str_test = "hello 1 2 1 2 hello bye"
.replace("hello", "bye")
.replace("1", "0")
.replace("2", "1");
println!("{:?}", str_test);Context
StackExchange Code Review Q#156540, answer score: 3
Revisions (0)
No revisions yet.