Skip to content

Comments

can add more edge if graph is directed#50

Open
alexTitova wants to merge 1 commit intomasterfrom
atitova
Open

can add more edge if graph is directed#50
alexTitova wants to merge 1 commit intomasterfrom
atitova

Conversation

@alexTitova
Copy link

изменен метод addEdge для добавление дуги-петли и обратной дуги для орг графа

@alexTitova alexTitova requested a review from svtz May 17, 2022 16:23
this.graphVisualizer.geometric.edges.push(new GeometricEdge(edge));
this.addEdgeToSVG(new GeometricEdge(edge));
this.updateSvg();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вот тут, наверное, нужен else и показать алерт, что что-то не в порядке!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

логично, странно, что не добавили алерт, а просто сделали вывод в консоль
добавлю

}
else{
console.log('Directed');
if (this.numberOfSelectedVertices() ===0 ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

странно, что эта проверка есть для ориентированного графа и её нет для неориентированного.
может, вытащить наверх?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

она есть, там идет проверка - что выбелено две вершины, иначе ошибка
alert('Для добавления ребра необходимо выбрать две вершины!') - строка 116

Comment on lines +177 to +202
// добавление ребра (именование вручную)
if (this.props.edgeNaming === true) {
let edgeName = prompt('Введите имя дуги:', '');
if (edgeName !== '' && edgeName !== null) {
edge = new DirectedEdge(new Vertex(this.vertexOne.name), new Vertex(this.vertexTwo.name), edgeName);
} else {
return;
}
} else {
// добавление ребра (автоматическое именование)
console.log('adding');
if (this.props.namedEdges == true) {
if (this.graphVisualizer.geometric.edges.length != 0) {
let edgeNumbers = [];
for (let i = 0; i < this.graphVisualizer.geometric.edges.length; i++) {
edgeNumbers[i] = Number(this.graphVisualizer.geometric.edges[i].edge.name);
}
let maxNum = Math.max.apply(null, edgeNumbers);
edge = new DirectedEdge(new Vertex(this.vertexOne.name), new Vertex(this.vertexTwo.name), (maxNum + 1).toString());
} else {
edge = new DirectedEdge(new Vertex(this.vertexOne.name), new Vertex(this.vertexTwo.name), '0');
}
} else {
// добавленеи неименованных ребер
edge = new DirectedEdge(new Vertex(this.vertexOne.name), new Vertex(this.vertexTwo.name));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Есть мнение, что вот этот кусок кода повторяет случай с рефлексивным ребром с точностью до имени второй вершины.
Может, сделать метод, который принимает на вход имяВершины1 и имяВершины2 вместо this.vertexOne/Two.name ?

this.graphVisualizer.geometric.edges.push(new GeometricEdge(edge));
this.addEdgeToSVG(new GeometricEdge(edge));
this.updateSvg();
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else if this.numberOfSelectedVertices() === 2
{
....
}
else alert(как в случае с неориентированным)

Comment on lines +164 to -117
console.log(edge);
this.props.graph.addEdge(edge);
this.graphVisualizer.geometric.edges.push(new GeometricEdge(edge));
this.addEdgeToSVG(new GeometricEdge(edge));
this.updateSvg();*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

выглядит как дублирующийся кусок кода, который тоже можно вытащить в отдельный метод

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants