Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have written simple code to construct a graph, but I think there are too many different types for such a simple task.

In this code I need to store Node structs in the Graph instead of pure values because it encapsulates some logic. The user should not know about Key. The user will be working with references to nodes.

How can I do this code simpler and clearer?

use std::collections::HashMap;
use std::rc::Rc;
use std::borrow::Borrow;

fn main() {
    let mut graph = Graph::<i32>::new();
    let n1 = graph.create_node(1);
    let n2 = graph.create_node(2);
    let n3 = graph.create_node(3);
    graph.add_edge(n1.clone(),n2.clone(), 6, false);
    graph.add_edge(n1.clone(),n3.clone(), 7, false);
    graph.get_rel_nodes(n1);
}

type Key = usize;
type Weight = usize;

#[derive(Debug, Clone)]
struct Node<T: Clone> {
    key: Key,
    value: T
}
impl<T: Clone> Node<T> {
    fn new(value: T, key: Key) -> Self {
        Node{
            key: key,
            value: value
        }
    }
}

#[derive(Debug)]
struct Graph<T: Clone> {
    map: HashMap<Key, HashMap<Key, Weight>>, // Contains edges
    list: HashMap<Key, Rc<Node<T>>>, // Contains nodes
    next_key: Key,
}

impl<T: Clone> Graph<T> {
    fn new() -> Self {
        Graph {
            map: HashMap::new(),
            list: HashMap::new(),
            next_key: 1,
        }
    }

    // Creates new node with value and return rel for it.
    fn create_node(&mut self, value: T) -> Rc<Node<T>>  {
        let key = self.get_next_key();
        let node = Node::new(value,key);
        let node = Rc::new(node);
        self.list.insert(key, node.clone());
        self.map.insert(key, HashMap::new());
        node
    }

    // Creates new adge between two nodes.
    fn add_edge<N: Borrow<Node<T>>>(&mut self, n1: N, n2: N, weight: Weight, bidirected: bool){
        let key1 = n1.borrow().key;
        let key2 = n2.borrow().key;
        if let Some(map) = self.map.get_mut(&key1) {
            map.insert(key2, weight);
        } else {
            return;
        }
        if bidirected {
            if let Some(map) = self.map.get_mut(&key2) {
                map.insert(key1, weight);
            }
        }
    }

    // Get relative nodes.
    fn get_rel_nodes<N: Borrow<Node<T>>>(&self, node: N) -> Vec<&Rc<Node<T>>> {
        let key = node.borrow().key;
        let rel_keys = self.map.get(&key).unwrap();
        let mut result = vec![];
        for(k,_) in rel_keys {
            result.push(self.list.get(k).unwrap());
        }
        result
    }

    fn get_next_key(&mut self) -> Key {
        let key = self.next_key;
        self.next_key += 1;
        key
    }
}
share|improve this question

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.