From 09958e1ee2848400b842ba90b854f3de2fd84b60 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Fri, 6 Jul 2018 22:23:23 -0700 Subject: [PATCH] Use `Map`s in `WeightConfig` (#497) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Now that `MapUtil` provides `toObject`/`fromObject`, we can keep storing weights in localStorage while representing them as ES6 `Map`s in memory. Here are some advantages: - The code is genuinely more typesafe. While writing this, I accidentally wrote `edgeWeights.get(key)`, where `edgeWeights` should have been `nodeWeights`. This was caught at compile time, and would not have been in the previous version. - Relatedly, the code now has zero `any`-casts as opposed to five. - The initialization of the default values is not abysmally ugly. - Whenever we iterate over these maps, (a) we can use `.entries()`, and (b) we don’t have to cast between string keys and semantic keys. This simplifies some of the control flow. - The extra null-checking on `get` forces us to either think about ways in which the check might fail, or reuse a previously fetched value that is known to be non-null (perhaps because it came from `entries`). - A particularly annoying Prettier line-break is avoided. :-) Here are some disadvantages: - The null-pipelining around the `rehydrate` function is a bit annoying. As @decentralion pointed out, what we want here is not a default value, but a default value and a function to transform a present value. This is Haskell’s `maybe : β → (α → β) → Maybe α → β` or Java’s `optional.map(fromObject).orElse(defaultValue)`. This commit implements one approach; another is to note that `fromObject` is invertible, writing `fromObject(LocalStore.get(k, toObject(d)))`. - That’s it, I think? Test Plan: I’ve tested that the sliders for both edge and node weights correctly influence the PageRank behavior, that the component is properly initialized with an empty localStorage, and that the component properly rehydrates from localStorage. wchargin-branch: weightconfig-maps --- src/app/credExplorer/WeightConfig.js | 209 ++++++++++++++------------- 1 file changed, 110 insertions(+), 99 deletions(-) diff --git a/src/app/credExplorer/WeightConfig.js b/src/app/credExplorer/WeightConfig.js index 22d7b5d..2f9798d 100644 --- a/src/app/credExplorer/WeightConfig.js +++ b/src/app/credExplorer/WeightConfig.js @@ -12,6 +12,7 @@ import { import {type EdgeEvaluator} from "../../core/attribution/pagerank"; import {byEdgeType, byNodeType} from "./edgeWeights"; import LocalStore from "./LocalStore"; +import * as MapUtil from "../../util/map"; // Hacks... import * as GithubNode from "../../plugins/github/nodes"; @@ -24,40 +25,36 @@ type Props = { }; // The key should be an EdgeAddressT, but a Flow bug prevents this. -type EdgeWeights = {[string]: UserEdgeWeight}; +type EdgeWeights = Map; type UserEdgeWeight = {|+logWeight: number, +directionality: number|}; const EDGE_WEIGHTS_KEY = "edgeWeights"; -const defaultEdgeWeights = (): EdgeWeights => ({ - [(GithubEdge._Prefix.authors: string)]: {logWeight: 0, directionality: 0.5}, - [(GithubEdge._Prefix.mergedAs: string)]: {logWeight: 0, directionality: 0.5}, - [(GithubEdge._Prefix.references: string)]: { - logWeight: 0, - directionality: 0.5, - }, - [(GithubEdge._Prefix.hasParent: string)]: {logWeight: 0, directionality: 0.5}, - [(GitEdge._Prefix.hasTree: string)]: {logWeight: 0, directionality: 0.5}, - [(GitEdge._Prefix.hasParent: string)]: {logWeight: 0, directionality: 0.5}, - [(GitEdge._Prefix.includes: string)]: {logWeight: 0, directionality: 0.5}, - [(GitEdge._Prefix.becomes: string)]: {logWeight: 0, directionality: 0.5}, - [(GitEdge._Prefix.hasContents: string)]: {logWeight: 0, directionality: 0.5}, -}); +const defaultEdgeWeights = (): EdgeWeights => + new Map() + .set(GithubEdge._Prefix.authors, {logWeight: 0, directionality: 0.5}) + .set(GithubEdge._Prefix.mergedAs, {logWeight: 0, directionality: 0.5}) + .set(GithubEdge._Prefix.references, {logWeight: 0, directionality: 0.5}) + .set(GithubEdge._Prefix.hasParent, {logWeight: 0, directionality: 0.5}) + .set(GitEdge._Prefix.hasTree, {logWeight: 0, directionality: 0.5}) + .set(GitEdge._Prefix.hasParent, {logWeight: 0, directionality: 0.5}) + .set(GitEdge._Prefix.includes, {logWeight: 0, directionality: 0.5}) + .set(GitEdge._Prefix.becomes, {logWeight: 0, directionality: 0.5}) + .set(GitEdge._Prefix.hasContents, {logWeight: 0, directionality: 0.5}); -// The key should be a NodeAddressT, but a Flow bug prevents this. -type NodeWeights = {[string]: UserNodeWeight}; +type NodeWeights = Map; type UserNodeWeight = number /* in log space */; const NODE_WEIGHTS_KEY = "nodeWeights"; -const defaultNodeWeights = (): NodeWeights => ({ - [(GithubNode._Prefix.repo: string)]: 0, - [(GithubNode._Prefix.issue: string)]: 0, - [(GithubNode._Prefix.pull: string)]: 0, - [(GithubNode._Prefix.review: string)]: 0, - [(GithubNode._Prefix.comment: string)]: 0, - [(GithubNode._Prefix.userlike: string)]: 0, - [(GitNode._Prefix.blob: string)]: 0, - [(GitNode._Prefix.commit: string)]: 0, - [(GitNode._Prefix.tree: string)]: 0, - [(GitNode._Prefix.treeEntry: string)]: 0, -}); +const defaultNodeWeights = (): NodeWeights => + new Map() + .set(GithubNode._Prefix.repo, 0) + .set(GithubNode._Prefix.issue, 0) + .set(GithubNode._Prefix.pull, 0) + .set(GithubNode._Prefix.review, 0) + .set(GithubNode._Prefix.comment, 0) + .set(GithubNode._Prefix.userlike, 0) + .set(GitNode._Prefix.blob, 0) + .set(GitNode._Prefix.commit, 0) + .set(GitNode._Prefix.tree, 0) + .set(GitNode._Prefix.treeEntry, 0); type State = { edgeWeights: EdgeWeights, @@ -75,10 +72,24 @@ export class WeightConfig extends React.Component { componentDidMount() { this.setState( - (state) => ({ - edgeWeights: LocalStore.get(EDGE_WEIGHTS_KEY, state.edgeWeights), - nodeWeights: LocalStore.get(NODE_WEIGHTS_KEY, state.nodeWeights), - }), + (state) => { + function rehydrate( + x: ?{[K]: V}, + default_: Map + ): Map { + return x ? MapUtil.fromObject(x) : default_; + } + return { + edgeWeights: rehydrate( + LocalStore.get(EDGE_WEIGHTS_KEY), + state.edgeWeights + ), + nodeWeights: rehydrate( + LocalStore.get(NODE_WEIGHTS_KEY), + state.nodeWeights + ), + }; + }, () => this.fire() ); } @@ -106,17 +117,21 @@ export class WeightConfig extends React.Component { fire() { const {edgeWeights, nodeWeights} = this.state; - LocalStore.set(EDGE_WEIGHTS_KEY, edgeWeights); - LocalStore.set(NODE_WEIGHTS_KEY, nodeWeights); - const edgePrefixes = Object.keys(edgeWeights).map((key) => { - const {logWeight, directionality} = edgeWeights[key]; - const prefix: EdgeAddressT = (key: any); - return {prefix, weight: 2 ** logWeight, directionality}; - }); - const nodePrefixes = Object.keys(nodeWeights).map((key) => ({ - prefix: ((key: any): NodeAddressT), - weight: 2 ** nodeWeights[key], - })); + LocalStore.set(EDGE_WEIGHTS_KEY, MapUtil.toObject(edgeWeights)); + LocalStore.set(NODE_WEIGHTS_KEY, MapUtil.toObject(nodeWeights)); + const edgePrefixes = Array.from(edgeWeights.entries()).map( + ([prefix, {logWeight, directionality}]) => ({ + prefix, + weight: 2 ** logWeight, + directionality, + }) + ); + const nodePrefixes = Array.from(nodeWeights.entries()).map( + ([prefix, logWeight]) => ({ + prefix, + weight: 2 ** logWeight, + }) + ); const edgeEvaluator = byNodeType(byEdgeType(edgePrefixes), nodePrefixes); this.props.onChange(edgeEvaluator); } @@ -127,57 +142,51 @@ class EdgeConfig extends React.Component<{ onChange: (EdgeWeights) => void, }> { weightControls() { - return Object.keys(this.props.edgeWeights).map((key) => { - const {logWeight} = this.props.edgeWeights[key]; - return ( - - ); - }); + return Array.from(this.props.edgeWeights.entries()).map(([key, datum]) => ( + + )); } directionControls() { - return Object.keys(this.props.edgeWeights).map((key) => { - const {directionality} = this.props.edgeWeights[key]; - return ( - - ); - }); + return Array.from(this.props.edgeWeights.entries()).map(([key, datum]) => ( + + )); } render() { return ( @@ -196,9 +205,8 @@ class NodeConfig extends React.Component<{ onChange: (NodeWeights) => void, }> { render() { - const controls = Object.keys(this.props.nodeWeights).map((key) => { - const currentValue = this.props.nodeWeights[key]; - return ( + const controls = Array.from(this.props.nodeWeights.entries()).map( + ([key, currentValue]) => ( - ); - }); + ) + ); return (

Node weights (in log space)