Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dijkstraクラスの作成 #9

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Dijkstraクラスの作成 #9

wants to merge 17 commits into from

Conversation

wheson
Copy link
Contributor

@wheson wheson commented Mar 31, 2018

Dijkstraクラスを作ってたものをプルリク送りました.改善点あれば修正します.

Dijkstra(int n, bool dir);
vector<long long> cost;
vector<int> prever;
void add_edge(int f, int t, long long c);
Copy link
Member

Choose a reason for hiding this comment

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

アッパーキャメルケースにしてください

vector<long long> cost;
vector<int> prever;
void add_edge(int f, int t, long long c);
bool has_path(int t); // tに至るパスはあるか
Copy link
Member

Choose a reason for hiding this comment

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

アッパーキャメルケースにしてください

vector<int> prever;
void add_edge(int f, int t, long long c);
bool has_path(int t); // tに至るパスはあるか
vector<int> get_shortest_path(int t);
Copy link
Member

Choose a reason for hiding this comment

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

アッパーキャメルケースにしてください

void add_edge(int f, int t, long long c);
bool has_path(int t); // tに至るパスはあるか
vector<int> get_shortest_path(int t);
void run(int f);
Copy link
Member

Choose a reason for hiding this comment

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

アッパーキャメルケースにしてください

private:
bool is_dir = false; // 無向グラフ: false, 有向グラフ: true
long long INFl = (long long)1e15;
int array_size_of_cost;
Copy link
Member

Choose a reason for hiding this comment

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

ローワーキャメルケースにしてください

#include "template.h"

struct Edge {
long long cost;
Copy link
Member

Choose a reason for hiding this comment

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

Int を使って下さい

struct Edge {
long long cost;
int to;
Edge(int t, long long c) {
Copy link
Member

Choose a reason for hiding this comment

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

Int を使って下さい

class Dijkstra {
private:
bool is_dir = false; // 無向グラフ: false, 有向グラフ: true
long long INFl = (long long)1e15;
Copy link
Member

Choose a reason for hiding this comment

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

Int を使って下さい

@wheson
Copy link
Contributor Author

wheson commented Mar 31, 2018

commitID:a37f441の時 Run関数内でfirst_state, sum_cost, current_stateという変数を宣言しているがテストに通っています

@odanado
Copy link
Member

odanado commented Mar 31, 2018

cpplintは変数名の形式をチェックしません

@wheson
Copy link
Contributor Author

wheson commented Mar 31, 2018

なるほど.目で確認するしかないのですね…

@odanado
Copy link
Member

odanado commented Mar 31, 2018

ググったら正規表現でチェックするツールが有りました
https://github.com/mapbox/cncc

@wheson
Copy link
Contributor Author

wheson commented Mar 31, 2018

long long をIntにするの,人によってかなり変わる気がするのでlong longでも良いとは思うのですが,どうなのでしょう

@odanado
Copy link
Member

odanado commented Mar 31, 2018

全部long longということ?

@wheson
Copy link
Contributor Author

wheson commented Mar 31, 2018

そうです.long longであれば誰であれ実行できると思うので

@odanado
Copy link
Member

odanado commented Apr 2, 2018

#10 の dinicと構造体Edgeの構造が異なるのが気になります(templateと非templateの違い)
構造体EdgeはGraphライブラリではよく使うので、 include ディレクトリ以下に graph.h を作ってそこで定義しておくのはどうでしょうか?

@wheson
Copy link
Contributor Author

wheson commented Apr 3, 2018

構造体Edgeは別でプルリクを出しておきます.

};

template <typename T>
Dijkstra<T>::Dijkstra(int n, bool dir)
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
Contributor Author

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