Is downcasting ok?

I have a situation where I have made this trait:

pub trait Transaction {
    fn selected(&mut self, values: &[Value]);
    fn set_error(&mut self, err: String);

    fn status_code(&mut self, _code: i64) { ... }
    fn header(&mut self, _name: &str, _value: &str) { ... }
    fn global(&self, _kind: i64) -> i64 { ... }
    fn arg(&mut self, _kind: i64, _name: &str) -> Rc<String> { ... }
    fn file_attr(&mut self, _fnum: i64, _atx: i64) -> Rc<String> { ... }
    fn file_content(&mut self, _fnum: i64) -> Arc<Vec<u8>> { ... }
    fn get_error(&mut self) -> String { ... }
    fn set_extension(&mut self, _ext: Box<dyn Any + Send + Sync>) { ... }
    fn get_extension(&mut self) -> Box<dyn Any + Send + Sync> { ... }
}

The methods I am wondering about are at the bottom, set_extension and get_extension, which allow access to some arbitrary data through the interface, which has to be downcast in order to access it.

Is this some kind of a standard pattern, or should perhaps I be re-thinking this?

The context is that the Transaction trait defines the input/output interface between the database and client, and although common cases can be anticipated, in some cases the client may want to be able to have additional data associated with the transaction which cannot be predicted. I think it makes sense, but downcasting seems a little "nasty" ( just some kind of feeling I have, no particular rationale to it ). If downcasting is "bad", is there a way to avoid it here?

In case it helps, here is an example where it is being used.

Assuming that the data has no common interface which you want to use (in which case you could use dyn Trait without downcasting), I assume you'll have to downcast as long as the type for the extension data is dynamic (i.e. can be different for each Transaction).

If the type will be known for several transactions, maybe it can be feasible to add a type parameter to Transaction and make it:

pub trait Transaction<E: Send + Sync> {
    fn selected(&mut self, values: &[Value]);
    fn set_error(&mut self, err: String);

    fn status_code(&mut self, _code: i64) { ... }
    fn header(&mut self, _name: &str, _value: &str) { ... }
    fn global(&self, _kind: i64) -> i64 { ... }
    fn arg(&mut self, _kind: i64, _name: &str) -> Rc<String> { ... }
    fn file_attr(&mut self, _fnum: i64, _atx: i64) -> Rc<String> { ... }
    fn file_content(&mut self, _fnum: i64) -> Arc<Vec<u8>> { ... }
    fn get_error(&mut self) -> String { ... }
    fn set_extension(&mut self, _ext: E) { ... }
    fn get_extension(&mut self) -> E> { ... }
}

Note that this has impact on the ability of using Transaction<E> as a dyn object in other places. And not sure if adding a type parameter is feasible in your case at all.

2 Likes

Yes, thanks for the suggestion, I had some vague inkling maybe I could use a type parameter but for some illogical reason have been shying away from it. Maybe that is the clean solution. On the other hand, maybe it incurs some costs? Hmm.

For arbitrary extensions, yes. The set of extensions is not fixed (like enum) and their interface is not guaranteed to be the same (like a specific trait).

Type parameter is very efficient if you use the trait only with a single type. If you use it with many types, all the methods will be duplicated for each type used in the type parameter. That adds to compilation time and binary size.

Another problem is object safety which limits use of dyn. You won't be able to have Box<dyn Transaction<T>>. It's only possible for specific parameter types Box<dyn Transaction<u32>>.

1 Like

I assume this can be worked around with, by adding a Self: Sized bound to get_extension and set_extension, right?

pub trait Transaction<E: Send + Sync> {
    /* … */
    fn set_extension(&mut self, _ext: E)
    where Self: Sized,
    { ... }
    fn get_extension(&mut self) -> E
    where Self: Sized
    { ... }
}

But then those two methods couldn't be called when you have a Box<dyn Transaction<T>>.

No, you just can't have a generic there. Rust would need some sort of higher-ranked-over-types type.

let _: Box<dyn for<T> Transaction<T>> = Box::new(todo!());
//             ^^^^^^ Not a thing currently
// error: only lifetime parameters can be used in this context
1 Like

Oh right, then the Self: Sized trick only works if the type parameter is a type parameter of a (few) method(s), and not of the trait itself.

Right, you can't exclude the trait type parameter itself somehow. The where Self: Sized bound is only useful for making methods not impede dyn-safety by making them not callable for dyn objects. Applying it to methods that take fresh generics is one such example. But it's not about generics per se, it's about limiting methods.

(E.g. you also can't use it on an associated constant to make a trait dyn-safe, or to "ignore" an associate type so it need not be specified, etc.)

At this point, my conclusion is that the downcasting solution is probably the best solution in this case.

The extension data is an obscure part of the interface, and may well not be used at all in typical applications. I think a type parameter would be potentially confusing, and would introduce unnecessary clutter ( and perhaps have other drawbacks ).

I don't think there are any performance gains to be had (from a type parameter). The downcasting will be a very rare event in the application I have at present, which is to signal that there is new email to be sent, and in any case I doubt it is a very expensive operation.

Thanks for all the responses though, I feel less uneasy about the design now.

1 Like

On a tangential note, in most cases, Rc<String>s is better represented as Rc<str> to avoid the double indirection since shared ownership doesn't really allow for mutation anyway.

3 Likes

I am using Rc::get_mut, it's not exactly true that Rc doesn't allow for mutation.

e.g. here : value.rs - source

    /// Append to a String.
    pub fn append(&mut self, val: &Value) {
        if let Value::String(s) = self {
            let val = val.str();
            if let Some(ms) = Rc::get_mut(s) {
                ms.push_str(&val);
            } else {
                let mut ns = String::with_capacity(s.len() + val.len());
                ns.push_str(s);
                ns.push_str(&val);
                *self = Value::String(Rc::new(ns));
            }
        } else {
            panic!()
        }
    }

( although whether I should be using str or String in this interface is still a question )

You should use Rc::make_mut() in this case, in order to simplify the code by manually cloning.

1 Like

Well, the code I have can specify the capacity of the String, potentially saving an allocation?

1 Like

Well, maybe? I'm not sure the added complexity is worth it though.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.