This is how I would write that code:
Rewritten
use web_sys::MouseEvent;
use web_sys::Element;
use msg::Msg;
// These should be scoped to the function unless they're used in multiple places.
use window_mouse::Msg as WindowMouseMsg;
use window_mouse::Action as WindowMouseAction;
use item_mouse::Msg as ItemMouseMsg;
use item_mouse::Action as ItemMouseAction;
use item_mouse::ItemType;
use viewport_mouse::Msg as ViewportMouseMsg;
use viewport_mouse::Action as ViewportMouseAction;
use web_sys_utils::get_data;
use web_sys_utils::find_parent;
fn handle_mouse_down(e: &MouseEvent) -> Option<Msg> {
e.stop_propagation();
let pos = get_mouse_point(e);
let pos_on_window = get_mouse_point_window(e);
let target = e.target().unwrap().unchecked_into::<Element>();
if let Some(draggable) = find_parent(target.clone(), "draggable") {
let node_id = get_data::<usize>(&draggable, "id")
.expect("Currently, every draggable element should have an id");
if draggable.class_list().contains("node") {
Some(Msg::ItemMouse(ItemMouseMsg {
pos,
action: ItemMouseAction::Down,
item_type: ItemType::NameDescriptionSpace,
id: node_id,
}))
} else if draggable.class_list().contains("node__linker") {
Some(Msg::WindowMouse(WindowMouseMsg {
pos: pos_on_window,
action: WindowMouseAction::DownOnLinker(node_id),
}))
} else {
unimplemented!("You can only drag nodes and linkers currently");
}
} else if let Some(relation) = find_parent(target, "relation") {
let relation_id = get_data::<usize>(&relation, "id")
.expect("Currently, every relation element should have an id");
Some(Msg::ItemMouse(ItemMouseMsg {
pos,
action: ItemMouseAction::Down,
item_type: ItemType::Relation,
id: relation_id,
}))
} else {
Some(Msg::ViewportMouse(ViewportMouseMsg {
pos,
action: ViewportMouseAction::Down,
}))
}
}
This is the rewrite that your proposed feature is supposed to improve. I argue that it doesn't, and that this code is as readable as any Rust code out there.
Rust already has a notoriety for line noise. It doesn't need another noisy syntax and inference mechanism to solve a superficial problem.
Which you've replaced with meaningless symbols. But those words aren't meaningless, they tell the reader what your data looks like and where to go to understand it. Hiding that does not make the code more readable, it just makes it more contextual and more terse.
Where in the code am I supposed to find out what a Msg::ViewportMouse
takes for an argument? I can see in your original code that it takes a viewport_mouse::Msg
. In your rewrite, I see that it takes a .
. Off to read the docs we go.
More over, things like Msg::ItemMouse(ItemMouseMsg { .. })
Should be replaced with Msg::from
calls. It is obviously trivial to convert an item_mouse::Msg
into a msg::Msg
, so implement that conversion. That gets rid of all of your redundant wrapping.
Overall, the point is thus: you have many ways to improve your code. Use them first, then after you can clearly demonstrate that none of them work, propose a language change.