Use NullUtil functions where appropriate (#506)

Summary:
The aesthetically nicest win is in `WeightConfig`. Other changes are
nice to have.

In many cases, we reduce the specificity of error messages thrown. For
instance, if an invariant was violated on an edge `e`, then we might
have thrown an error with message `EdgeAddress.toString(e.address)`. But
we did so not because we thought that this was genuinely worth it, but
only because we were forced to explicitly throw an error at all. These
errors should never be hit, anyway, so we don’t feel bad about replacing
these with errors that are simply the string `"null"` or `"undefined"`,
as appropriate.

Test Plan:
Running `yarn travis --full` passes, and the cred explorer still seems
to work with both populated and empty `localStorage`.

wchargin-branch: use-null-util
This commit is contained in:
William Chargin 2018-07-09 17:53:05 -07:00 committed by GitHub
parent 4af8ff2471
commit bb7b538f44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 37 additions and 88 deletions

View File

@ -13,6 +13,8 @@ import type {PluginAdapter} from "../pluginAdapter";
import {type EdgeEvaluator} from "../../core/attribution/pagerank";
import {WeightConfig} from "./WeightConfig";
import * as NullUtil from "../../util/null";
type Props = {};
type State = {
repoOwner: string,
@ -121,8 +123,8 @@ export default class App extends React.Component<Props, State> {
)}
<WeightConfig onChange={(ee) => this.setState({edgeEvaluator: ee})} />
<PagerankTable
graph={graphWithMetadata ? graphWithMetadata.graph : null}
adapters={graphWithMetadata ? graphWithMetadata.adapters : null}
graph={NullUtil.map(graphWithMetadata, (x) => x.graph)}
adapters={NullUtil.map(graphWithMetadata, (x) => x.adapters)}
pagerankResult={pagerankResult}
/>
</div>

View File

@ -13,6 +13,7 @@ import {type EdgeEvaluator} from "../../core/attribution/pagerank";
import {byEdgeType, byNodeType} from "./edgeWeights";
import LocalStore from "./LocalStore";
import * as MapUtil from "../../util/map";
import * as NullUtil from "../../util/null";
// Hacks...
import * as GithubNode from "../../plugins/github/nodes";
@ -73,19 +74,13 @@ export class WeightConfig extends React.Component<Props, State> {
componentDidMount() {
this.setState(
(state) => {
function rehydrate<K: string, V>(
x: ?{[K]: V},
default_: Map<K, V>
): Map<K, V> {
return x ? MapUtil.fromObject(x) : default_;
}
return {
edgeWeights: rehydrate(
LocalStore.get(EDGE_WEIGHTS_KEY),
edgeWeights: NullUtil.orElse(
NullUtil.map(LocalStore.get(EDGE_WEIGHTS_KEY), MapUtil.fromObject),
state.edgeWeights
),
nodeWeights: rehydrate(
LocalStore.get(NODE_WEIGHTS_KEY),
nodeWeights: NullUtil.orElse(
NullUtil.map(LocalStore.get(NODE_WEIGHTS_KEY), MapUtil.fromObject),
state.nodeWeights
),
};

View File

@ -8,6 +8,8 @@ import {
} from "../../core/graph";
import type {EdgeEvaluator} from "../../core/attribution/pagerank";
import * as NullUtil from "../../util/null";
export function byEdgeType(
prefixes: $ReadOnlyArray<{|
+prefix: EdgeAddressT,
@ -16,13 +18,9 @@ export function byEdgeType(
|}>
): EdgeEvaluator {
return function evaluator(edge: Edge) {
const datum = prefixes.find(({prefix}) =>
EdgeAddress.hasPrefix(edge.address, prefix)
const {weight, directionality} = NullUtil.get(
prefixes.find(({prefix}) => EdgeAddress.hasPrefix(edge.address, prefix))
);
if (datum == null) {
throw new Error(EdgeAddress.toString(edge.address));
}
const {weight, directionality} = datum;
return {
toWeight: directionality * weight,
froWeight: (1 - directionality) * weight,

View File

@ -1,8 +1,9 @@
// @flow
import {type Edge, type Graph, type NodeAddressT, NodeAddress} from "../graph";
import {type Edge, type Graph, type NodeAddressT} from "../graph";
import type {Distribution, SparseMarkovChain} from "./markovChain";
import * as MapUtil from "../../util/map";
import * as NullUtil from "../../util/null";
export type Probability = number;
export type Contributor =
@ -71,21 +72,12 @@ export function createContributions(
target: NodeAddressT,
contribution: Contribution
) {
const contributions = result.get(target);
if (contributions == null) {
// Should be impossible based on graph invariants.
throw new Error("missing target: " + NodeAddress.toString(target));
}
const contributions = NullUtil.get(result.get(target));
(((contributions: $ReadOnlyArray<Contribution>): any): Contribution[]).push(
contribution
);
const source = contributorSource(target, contribution.contributor);
const priorOutWeight = totalOutWeight.get(source);
if (priorOutWeight == null) {
// Should be impossible based on graph invariants.
throw new Error("missing source: " + NodeAddress.toString(source));
}
const priorOutWeight = NullUtil.get(totalOutWeight.get(source));
totalOutWeight.set(source, priorOutWeight + contribution.weight);
}
@ -115,11 +107,7 @@ export function createContributions(
for (const [target, contributions] of result.entries()) {
for (const contribution of contributions) {
const source = contributorSource(target, contribution.contributor);
const normalization = totalOutWeight.get(source);
if (normalization == null) {
// Should be impossible.
throw new Error("missing node: " + NodeAddress.toString(source));
}
const normalization = NullUtil.get(totalOutWeight.get(source));
const newWeight: typeof contribution.weight =
contribution.weight / normalization;
// (any-cast because property is not writable)
@ -139,7 +127,7 @@ function createNodeAddressMarkovChain(
const source = contributorSource(target, contribution.contributor);
inNeighbors.set(
source,
contribution.weight + (inNeighbors.get(source) || 0)
contribution.weight + NullUtil.orElse(inNeighbors.get(source), 0)
);
}
return inNeighbors;
@ -157,22 +145,14 @@ function nodeAddressMarkovChainToOrderedSparseMarkovChain(
return {
nodeOrder,
chain: nodeOrder.map((dst) => {
const theseNeighbors = chain.get(dst);
if (theseNeighbors == null) {
// Should be impossible.
throw new Error("missing key: " + NodeAddress.toString(dst));
}
const theseNeighbors = NullUtil.get(chain.get(dst));
const result = {
neighbor: new Uint32Array(theseNeighbors.size),
weight: new Float64Array(theseNeighbors.size),
};
let i = 0;
for (const [src, weight] of theseNeighbors.entries()) {
const srcIndex = addressToIndex.get(src);
if (srcIndex == null) {
// Should be impossible.
throw new Error("missing neighbor: " + NodeAddress.toString(src));
}
const srcIndex = NullUtil.get(addressToIndex.get(src));
result.neighbor[i] = srcIndex;
result.weight[i] = weight;
i++;

View File

@ -2,7 +2,7 @@
import sortBy from "lodash.sortby";
import {type NodeAddressT, NodeAddress} from "../graph";
import type {NodeAddressT} from "../graph";
import {
type Contribution,
type NodeToContributions,
@ -10,6 +10,7 @@ import {
} from "./graphToMarkovChain";
import type {NodeDistribution} from "./pagerank";
import * as MapUtil from "../../util/map";
import * as NullUtil from "../../util/null";
export type ScoredContribution = {|
+contribution: Contribution,
@ -33,18 +34,12 @@ export function decompose(
contributions: NodeToContributions
): PagerankNodeDecomposition {
return MapUtil.mapValues(contributions, (target, contributions) => {
const score = pr.get(target);
if (score == null) {
throw new Error("missing target: " + NodeAddress.toString(target));
}
const score = NullUtil.get(pr.get(target));
const scoredContributions = sortBy(
contributions.map(
(contribution): ScoredContribution => {
const source = contributorSource(target, contribution.contributor);
const sourceScore = pr.get(source);
if (sourceScore == null) {
throw new Error("missing source: " + NodeAddress.toString(source));
}
const sourceScore = NullUtil.get(pr.get(source));
const contributionScore = contribution.weight * sourceScore;
return {contribution, source, sourceScore, contributionScore};
}

View File

@ -4,8 +4,9 @@ import deepEqual from "lodash.isequal";
import sortBy from "lodash.sortby";
import {makeAddressModule, type AddressModule} from "./address";
import {toCompat, fromCompat, type Compatible} from "../util/compat";
import * as NullUtil from "../util/null";
export opaque type NodeAddressT: string = string;
export opaque type EdgeAddressT: string = string;
export const NodeAddress: AddressModule<NodeAddressT> = (makeAddressModule({
@ -383,11 +384,8 @@ export class Graph {
}
} else {
this._edges.set(edge.address, edge);
const inEdges = this._inEdges.get(edge.dst);
const outEdges = this._outEdges.get(edge.src);
if (inEdges == null || outEdges == null) {
throw new Error(`Invariant violation on edge ${edgeToString(edge)}`);
}
const inEdges = NullUtil.get(this._inEdges.get(edge.dst));
const outEdges = NullUtil.get(this._outEdges.get(edge.src));
inEdges.push(edge);
outEdges.push(edge);
}
@ -402,11 +400,8 @@ export class Graph {
const edge = this._edges.get(address);
if (edge != null) {
this._edges.delete(address);
const inEdges = this._inEdges.get(edge.dst);
const outEdges = this._outEdges.get(edge.src);
if (inEdges == null || outEdges == null) {
throw new Error(`Invariant violation on ${edgeToString(edge)}`);
}
const inEdges = NullUtil.get(this._inEdges.get(edge.dst));
const outEdges = NullUtil.get(this._outEdges.get(edge.src));
// TODO(perf): This is linear in the degree of the endpoints of the
// edge. Consider storing in non-list form (e.g., `_inEdges` and
// `_outEdges` could be `Map<NodeAddressT, Set<EdgeAddressT>>`).
@ -502,21 +497,11 @@ export class Graph {
const direction = options.direction;
const adjacencies: {edges: Edge[], direction: string}[] = [];
if (direction === Direction.IN || direction === Direction.ANY) {
const inEdges = this._inEdges.get(node);
if (inEdges == null) {
throw new Error(
`Invariant violation: No inEdges for ${NodeAddress.toString(node)}`
);
}
const inEdges = NullUtil.get(this._inEdges.get(node));
adjacencies.push({edges: inEdges, direction: "IN"});
}
if (direction === Direction.OUT || direction === Direction.ANY) {
const outEdges = this._outEdges.get(node);
if (outEdges == null) {
throw new Error(
`Invariant violation: No outEdges for ${NodeAddress.toString(node)}`
);
}
const outEdges = NullUtil.get(this._outEdges.get(node));
adjacencies.push({edges: outEdges, direction: "OUT"});
}
@ -564,11 +549,8 @@ export class Graph {
});
const sortedEdges = sortBy(Array.from(this.edges()), (x) => x.address);
const indexedEdges = sortedEdges.map(({src, dst, address}) => {
const srcIndex = nodeToSortedIndex.get(src);
const dstIndex = nodeToSortedIndex.get(dst);
if (srcIndex == null || dstIndex == null) {
throw new Error(`Invariant violation`);
}
const srcIndex = NullUtil.get(nodeToSortedIndex.get(src));
const dstIndex = NullUtil.get(nodeToSortedIndex.get(dst));
return {srcIndex, dstIndex, address: EdgeAddress.toParts(address)};
});
const rawJSON = {

View File

@ -6,6 +6,7 @@ import {
createExampleRepo,
createExampleSubmoduleRepo,
} from "../example/exampleRepo";
import * as NullUtil from "../../../util/null";
function parseArgs() {
const argv = process.argv.slice(2);
@ -31,13 +32,9 @@ function parseArgs() {
}
}
if (target == null) {
fail();
throw new Error("Unreachable"); // for Flow
}
return {
submodule,
target: (target: string),
target: NullUtil.get(target),
};
}