Too many possibilities for the same, is it good?

1

Rust is an interesting language, however it gives too many possibilities doing the same stuff, for example:
1.

"write" => {
                let fname = self.parameter(&log, 0, fun_block, res_prev);
                let file = File::create(&*fname);
                if file.is_ok() {
                    let mut file = file.unwrap();
                    let len = fun_block.params.len();
                    for  i in 1..len {
                       if write!(file, "{}", self.parameter(&log, i, fun_block, res_prev)).is_err() {
                            log.error(&format!{"Writing in {} failed at {}", fname, &fun_block.script_line});
                            break
                        }
                    }
                } else {
                    log.error(&format!{"File {} can't be opened for writing at {}", fname, &fun_block.script_line});
                } 
            }

            "writea" => {
                let fname = self.parameter(&log, 0, fun_block, res_prev);
                if let Ok(mut file) =  OpenOptions::new()
                    .read(true)
                    .append(true) 
                    .create(true)
                    .open(&*fname) {
                    let len = fun_block.params.len();
                    for i in   1.. len {
                        if write!(file, "{}", self.parameter(&log, i, fun_block, res_prev)).is_err() {
                            log.error(&format!{"Writing in {} failed at {}", fname, &fun_block.script_line});
                            break
                        }
                    }
                } else {
                    log.error(&format!{"File {} can't be opened for writing at {}", fname, &fun_block.script_line});
                } 
            }

            "writex" if cfg!(unix) => {
                let fname = self.parameter(&log, 0, fun_block, res_prev);
                match OpenOptions::new()
                    .create(true)
                    .write(true)
                    .truncate(true)
                    .mode(0o700)
                    .open(&*fname) {
                    Ok(mut file) => {
                        let len = fun_block.params.len();
                        for  i in 1..len {
                        if write!(file, "{}", self.parameter(&log, i, fun_block, res_prev)).is_err() {
                                log.error(&format!{"Writing in {} failed at {}", fname, &fun_block.script_line});
                                break
                            }
                        }
                    } 
                    Err(_) => log.error(&format!{"File {} can't be opened for writing at {}", fname, &fun_block.script_line}),
                } 
            }

You can propose more variants, but question is - which one do you prefer and why? I just try to create a particular style of coding to get it more consistent and easy to read.

2

If I understand correctly simplified versions of you examples would be:

// 1
fn foo<T, E>(x: Result<T, E>){
    if x.is_ok() {
        let x = x.unwrap();
        process_ok(x);
    } else {
        process_err();
    }
}

// 2
fn foo<T, E>(x: Result<T, E>){
    if let Ok(x) = x {
        process_ok(x);
    } else {
        process_err();
    }
}

// 3
fn foo<T, E>(x: Result<T, E>){
    match x {
        Ok(x) => process_ok(x),
        Err(_) => process_err(),
    }
}

Here the first example is definitively unidiomatic. There is even specific clippy lint that checks for patter of if x.is_some() { x.unwrap() } (if you don't use clippy yet, you should; it's great).

Examples 2. and 3. are indeed equivalent. Whether you choose one style or the other is mostly a preference. Some people prefer match statements, and some do not.

Now when it comes to File::create vs OpenOptions all of your examples do different things. Also if cfg!(unix) guard is unique to only one example, so I cannot compare your examples based on that.

On the final note. Yes there are some things that you can express in different ways in Rust, but I haven't found it overwhelming. And even if somebody chooses a different style I have no problem with reading and understanding their code.

9 Likes
3

Run cargo fmt and cargo clippy first.

I expect clippy will agree, but 1 is not recommended:

Is an unnecessary unwrap when we could do 2.

As for whether to use OpenOptions or platform specific code. Depends on what the program needs to do. Check the docs for more info.

All 3 versions behave differently so it's not exactly fair to say they are different ways of writing the same thing.

EDIT: Just read the first reply, I didn't even notice the use of match vs if let at first :joy:

2 Likes
4

You relay on what clippy or rustfmt suggests most. I can't tell that it's bad. But it is an opinion of people created that programs. I am more interested what do you use and why. So quick summary will be - I use what what clippy suggest me, because I believe that people who created clippy are smart enough. But still, if Rust offers alternatives, there should be some reason .

5

I guess there might be circumstances where one wants to know if an operation succeeded or failed but does not care about any other result of it. In which case this:

    if x.is_ok() {...

would be appropriate.

1 Like
6

Clippy's lints have section explaining why you should use idiom they enforce. Unfortunately lint that I have linked clippy:unnecessary_unwrap has unhelpful information:

Why is this bad?

Using if let or match is more idiomatic.

So to explain why checking variant if it is Some or Ok, and then calling unwrap is unidiomatic, when you write following code:

let x: Option<()> = todo!();
if x.is_some() {
    let inner = x.unwrap();
    process(inner);
}

It is true that this works. And compiler will probably optimize out another check in unwrap (and eliminate fully panic path). However there are many problems with this approach:

  • This is simply longer. You are using 2 lines of code, but you could express that easily with only one.
  • unwrap usually means that some situation is impossible. It catches eye and suggest that some condition requires special care in the review. However in this example it is quite normal that x could be None. It cannot be None in this code path, but in general this is quite expacted.
  • Similar argument goes for is_some check. This usually means "I only care about what variant this Option is, and not what is the inner value".
  • It worsens local reasoning. To analyze if this unwrap is correct you have to hold additional context, the fact that you have already checked before that Option is Some. In this example you could argue that this is quite easy. Those two lines are adjacent. But there could be more code between them.
  • Perhaps most importantly, this is very prone to refactor errors. If you change if-condition, but don't update unwrapping code, then suddenly your code will panic at runtime.

Consider more idiomatic approach:

let x: Option<()> = todo!();
if let Some(inner) = x {
    process(inner);
}

This code is shorter, expresses intent directly and prevents many errors that could be introduced during refactoring. That is why this code is idiomatic.

8 Likes
7

At the risk of being an old grumpy developer (which I am) "good programming" is external to "what I program in".

I'd say neither of those variations are good because they intermingle different abstractions. Looping, opening files, writing to files, error handling etc. are all different levels of abstractions and arguably [sh|c]ould be separate fns which would significantly aid readability.

Being explicit, I don't think the problem here is rust so much as the problem here is "good programming practices" itself :slight_smile:

I'm going to put on my now in preparation for the responses.

8 Likes
8

Absolutely. Using functions enables the question mark operator together with the Result type, something like this:

impl SomeUnknownStruct {
    fn something(&self, ...) {
        match ... {
            "writea" => self.write_file(&self, ...),
          }
    }
    fn write_file(&self, ...) {
        let filename = self.parameter(&log, 0, fun_block, res_prev);
        if let Err(msg) = self.write_to_filename(filename, ...) {
            log.error(&msg);
        }
    }
    fn write_to_filename(&self, filename: &str, ...) -> Result<(), String> {
        let mut file = OpenOptions::new()
            .read(true)
            .append(true)
            .create(true)
            .open(filename)
            .map_err(|_| {
                format!(
                    "File {filename} can't be opened for writing at {line}",
                    line = fun_block.script_line
                )
            })?;

        for i in 1..fun_block.params.len() {
            file.write_all(self.parameter(log, i, fun_block, res_prev).as_bytes())
                .map_err(|_| {
                    format!(
                        "Writing in {filename} failed at {line}",
                        line = fun_block.script_line
                    )
                })?;
        }

        Ok(())
    }
}

Maybe the whole code base could be reworked relative easily by using Result heavily and introducing a custom Error enum. For example, maybe passing around the log-thing will vanish then.

9

Clippy and rustfmt are officially part of the Rust project. They are developed to the same standard of quality as Rust itself, involve many of the same people, and employ the same consensus-based development principles as the rest of the language. They are also relied on by most people using the language.

They are the best approximation to a machine-encoded community consensus on the issues they cover. Yes, they are occasionally overzealous, and sometimes have bugs, but unless you have a specific reason to do otherwise the default is to follow their suggestions.

15 Likes
10

In your first version you use File::create(&*fname);. This is a shorter, more convenient and more readable way compared to OpenOptions::new().write(true).create(true).truncate(true).open(&*fname). So it makes sense to offer create for one of the most common cases on how to open / create a file.
On the other hand, you might want to do something different (and less common probably) you still can do it with OpenOptions::new()...

You also use

if file.is_ok() {
    let mut file = file.unwrap()

which is better expressed with

if let Ok(file) = file {
   ...

but there might be cases, where you're only interested in whether file.is_ok() and you don't want to unwrap(), so it makes sense to offer is_ok(). Or you might be sure that file.is_ok(), so you just unwarp()

Rust is offering these things, because it makes sense in different scenarios.

I'm also quite convinced if you would "clean up" the language from everything that makes different ways of doing something possible, you'll end up with an extremely crippled language.

2 Likes
11

I think it's also important to which level of a programmer you address your suggestion. If the person programs using Rust occasionally and mostly using another language, then a construction as 'if result.is_ok() {
.... result.unwrap();
...' is more suitable than 'if let Ok(result) = result {'
because first version will be more readable for him/her .

I also agree that '?' is a good solution , similar to other languages - throw an exception.... However it will require a significant redesign code, which can be wasting time. So it is better to program more in the Rust way from begining. BTW, '?' processed by Kotlin differently, it just stopping evaluation the expression, however doesn't leave the current block .

Interesting, for example that other languages started follow Rust and now you can use a similar to 'if let' in Java.

Regarding Clippy, for example, Kotlin tells me - do not use 'if (something == false)', use 'if (!something)'. Ironically, using 'if (something == false)' suggested me my student, he explained that '!' can be easily overlooked, and meaning of 'if' become wrong for you . So it is why I am sceptical to Clippy suggestions too .

12

"if let" is not an advanced feature. It is worth learning even for beginners, since it will be used frequently in almost all Rust programs.

I think you may be in a different situation teaching Rust in a classroom, compared to many people here who are learning in order to become Rust programmers. But by reading responses here you'll see a strong consensus to follow the clippy hints, avoid unwrap, etc. You will be helping your students if you follow this consensus, since this will be expected of them if they do anything further with Rust outside of class.

7 Likes
13

Well, most of all 'unwrap()' might panic, contrary to 'if let'
But some people don't care to write robust code. Others do.

14

Rust is interesting, but it has too many ways to express similar things, e.g. [...]

There are too many approaches to achieve the same outcome in rust, even though it's an interesting language. Here are some examples: [...]

Lately I've been interested in Rust, but one thing that puts me off is that there is not one obvious way achieve a particular task, but many. Examples below [...]

...

2 Likes
15

That is a common misconception, ? is not the same as "throw an exception", and its also not a "anti pattern" or "not the rust way". It is very much the rust way, in the sense that it is simply a sugar for early returning your error type. If your function could trigger multiple errors, and you want the caller to decide what to do about it, then you need to return an error type that represents all of those erros, I don't see it as "wasting time", its doing the work that needs to be done. Besides, if you use with functions like map_err you can go from one error type to another pretty smoothly, just a bunch of result.map_err(...)?.

If you are in a context where it doesn't matter if the application just crashes, then you can just unwrap (which is closer to just throwing an exception).

16

Really? Have you ever used Perl or Scala? Heck any problem has many ways of attack even in C or C++.

5 Likes
17

"Hardest - HTML"
From a reddit thread about the easiest and hardest programming language to learn.
:rofl:

18

Thanks to everyone for providing a valuable input. I overlooked [quote="jonnyso, post:15, topic:118150"]
result.map_err(...)?
[/quote] to change Result type. It was easy for OOP languages, but a bit unclear how to do that in Rust.
I also found that closures are very useful for reducing boilerplate code without introducing additional functions or macros. I should also wider using them. Finally some fun example how I use Rust to defeat other languages:

C#
C#1920×892 99.2 KB

How would you do the same in Rust?

    let number = 1;
    
    let result = ["zero", "one", "two", "Default Case"][if number > 2 /*|| number < 0*/ {3} else {number}];
    println!{"result: {result}"}

My C++ friend told - your Rust code isn't readable.
Maybe, but it's a very intuitive to write.

19

Improvement to your code:

let result = [
    "zero",
    "one",
    "two",
]
    .get(number)
    .unwrap_or(&"Default Case");

This is an improvement because the hardcoded number 3 is eliminated and because that reduces the amount of changes needed to add a branch that prints result: three. Compare:

 let result = [
     "zero",
     "one",
     "two",
+    "three",
     "Default Case"
 ]
-    [if number > 2 || number < 0 {3} else {number}];
+    [if number > 3 || number < 0 {4} else {number}];

 let result = [
     "zero",
     "one",
     "two",
+    "three",
 ]
     .get(number)
     .unwrap_or(&"Default Case");
2 Likes
20

Awesome!