HiveBrain v1.2.0
Get Started
← Back to all entries
debugrustMinor

Netscape bookmark file generator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
netscapefilebookmarkgenerator

Problem

I started to learn Rust some time ago and I wonder if there is some way to write Rust code that is both concise and safe — in particular, avoiding unwrap(), casting functions and try!().

Let's see an example. I wrote a software to generate random bookmarks and save them to a file, both in Rust and Haskell to compare.

First, I'll show you the Haskell version:

import Control.Monad (liftM2, replicateM)
import System.Environment (getArgs)
import System.Random (newStdGen, randomRs)

main :: IO ()
main = do
    args  writeFile outputFile fileContent
        Nothing -> putStrLn "Usage: ./GenBookmarks outputfile.html [count]"

generateFile :: [String] -> IO (Maybe (String, String))
generateFile [outputFile, count] = do
    fileContent  IO String
generateFileContent count = do
    bookmarks "
            , ""
            , "Bookmarks"
            , "Menu des marque-pages"
            , ""
            ]
            ++ bookmarks
    return $ unlines fileLines

generateBookmarks :: Int -> IO [String]
generateBookmarks count =
    liftM2 (++) (replicateM count generateDirectory) (replicateM (count * 4) generateBookmark)
    where generateDirectory = do
                directoryName " ++ directoryName ++ ""
          generateBookmark = do
                bookmarkName \n" ++ bookmarkName ++ "\n"

generateName :: IO String
generateName = do
    g <- newStdGen
    return $ take 10 $ randomRs ('a', 'z') g


Here is the Rust version:

```
extern crate rand;

use std::env;
use std::fs::File;
use std::io::Write;

use rand::Rng;

fn options() -> Option {
match env::args().count() {
2 => Some ((env::args().nth(1).unwrap(), 10)),
3 => Some (
(
env::args().nth(1).unwrap(),
env::args().nth(2).unwrap().parse().unwrap(),
)
),
_ => None,
}
}

fn generate_name() -> String {
rand::thread_rng().gen_ascii_chars().take(10).collect()
}

fn write_data(file: &mut File, count: i32) -> Result

Solution

Overall, your code seems pretty reasonable. Let's look at your specific questions:


The options() function looks really bad and unsafe

Let's get a piece of terminology out of the way: in Rust, the word
"unsafe" has a very specific meaning — memory unsafety. This code
has no memory safety issues.

It does have some ugliness though, mostly around the use of
unwrap. unwrap will crash the program if it is None, so it's
much nicer to avoid that when possible. Instead of checking the length
and then trusting that values are there, we can just pull some values
off the iterator and then pattern match on the results.

Option has methods like map and and_then, which correspond to option monad methods you may already be familiar with from Haskell. We use unwrap_or at the end to give a default value when the count is None, but this will never cause a panic.

The better solution would be to use a third party library for command
line argument parsing, but this will do for a quick solution.


file writing (with all these try!) looks bad.

This is true, but does highlight the possible failures much more than
a language like C or C++ might. Unfortunately, Rust does not have the
monadic do syntax that can be used to clean this up in Haskell,
try! is it for the moment.


to_string(), as_bytes() and b"some text" is cumbersome.

Agreed. To solve this, I'd suggest using the write! and writeln!
macros. These combine formatting strings and writing them to a given
formatter.

Other changes

  • There's no reason to support a negative number of lines; use an unsigned value.



  • There's no reason to tie the inner function to a File; any type that implements Write will do.



  • Use the type alias for io::Result to avoid redundancy



  • There's no reason to take a String argument; string slices (&str) are more flexible.



  • There's no reason to create an allocated vector; an array on the stack is more efficient.



Final code

extern crate rand;

use std::env;
use std::fs::File;
use std::io::{self, Write};

use rand::Rng;

fn options() -> Option {
    let mut args = env::args();
    args.next(); // Skip the program argument

    match (args.next(), args.next()) {
        (Some(file), count) => {
            let count = count.and_then(|c| c.parse().ok()).unwrap_or(10);
            Some((file, count))
        },
        _ => None,
    }
}

fn generate_name() -> String {
    rand::thread_rng().gen_ascii_chars().take(10).collect()
}

fn write_data(mut out: W, count: u32) -> io::Result
    where W: io::Write
{
    for _ in 1..count {
        try!(write!(out, "{}", generate_name()));
    }
    for _ in 1..count * 4 {
        try!(write!(out, "\n{0}\n\n", generate_name()));
    }
    Ok(())
}

fn generate_file(outputfile: &str, count: u32) -> io::Result {
    let data = [
        "",
        "",
        "Bookmarks",
        "Menu des marque-pages",
        "",
    ];
    let mut file = try!(File::create(outputfile));
    for line in &data {
        try!(writeln!(file, "{}", line));
    }
    try!(write_data(file, count));
    Ok(())
}

fn main() {
    match options() {
        Some((outputfile, count)) => {
            match generate_file(&outputfile, count) {
                Ok(()) => (),
                Err(error) => println!("{}", error),
            }
        },
        None => println!("Usage: ./GenBookmarks outputfile.html [count]"),
    }
}

Code Snippets

extern crate rand;

use std::env;
use std::fs::File;
use std::io::{self, Write};

use rand::Rng;

fn options() -> Option<(String, u32)> {
    let mut args = env::args();
    args.next(); // Skip the program argument

    match (args.next(), args.next()) {
        (Some(file), count) => {
            let count = count.and_then(|c| c.parse().ok()).unwrap_or(10);
            Some((file, count))
        },
        _ => None,
    }
}

fn generate_name() -> String {
    rand::thread_rng().gen_ascii_chars().take(10).collect()
}

fn write_data<W>(mut out: W, count: u32) -> io::Result<()>
    where W: io::Write
{
    for _ in 1..count {
        try!(write!(out, "<DT><H3 ADD_DATE=\"1438910135\" LAST_MODIFIED=\"1438910135\">{}</H3>", generate_name()));
    }
    for _ in 1..count * 4 {
        try!(write!(out, "<DL><p>\n<DT><A HREF=\"https://{0}.com/\">{0}</a>\n</DL><p>\n", generate_name()));
    }
    Ok(())
}

fn generate_file(outputfile: &str, count: u32) -> io::Result<()> {
    let data = [
        "<!DOCTYPE NETSCAPE-Bookmark-file-1>",
        "<META HTTP-EQUIV=\"Content-Type\" CONTENT=\"text/html; charset=UTF-8\">",
        "<TITLE>Bookmarks</TITLE>",
        "<H1>Menu des marque-pages</H1>",
        "<DL><p>",
    ];
    let mut file = try!(File::create(outputfile));
    for line in &data {
        try!(writeln!(file, "{}", line));
    }
    try!(write_data(file, count));
    Ok(())
}

fn main() {
    match options() {
        Some((outputfile, count)) => {
            match generate_file(&outputfile, count) {
                Ok(()) => (),
                Err(error) => println!("{}", error),
            }
        },
        None => println!("Usage: ./GenBookmarks outputfile.html [count]"),
    }
}

Context

StackExchange Code Review Q#101621, answer score: 5

Revisions (0)

No revisions yet.