Why this implementation of Bellman-Ford doesn't work?

Question

The following example contains a negative cycle, and yet the program doesn't seem to find it. Can someone point out what is wrong? It is supposed to print out a negative cycle if one exists, but the program doesn't do what is expected.

#include <iostream>

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>

#include <math.h>

using namespace std;

// a structure to represent a weighted edge in graph
struct Edge
{
    int src, dest, weight;
};

// a structure to represent a connected, directed and weighted graph
struct Graph
{
    // V-> Number of vertices, E-> Number of edges
    int V, E;

    // graph is represented as an array of edges.
    struct Edge* edge;
};

// Creates a graph with V vertices and E edges
struct Graph* createGraph(int V, int E)
{
    struct Graph* graph = (struct Graph*) malloc( sizeof(struct Graph) );
    graph->V = V;
    graph->E = E;

    graph->edge = (struct Edge*) malloc( graph->E * sizeof( struct Edge ) );

    return graph;
}

// A utility function used to print the solution
void printArr(int dist[], int n)
{
    printf("Vertex   Distance from Source\n");
    for (int i = 0; i < n; ++i)
        printf("%d \t\t %d\n", i, dist[i]);
}

// The main function that finds shortest distances from src to all other
// vertices using Bellman-Ford algorithm.  The function also detects negative
// weight cycle
void BellmanFord(struct Graph* graph, int src)
{
    int V = graph->V;
    int E = graph->E;
    int dist[V];

    // Step 1: Initialize distances from src to all other vertices as INFINITE
    for (int i = 0; i < V; i++)
        dist[i]   = INT_MAX;
    dist[src] = 0;

    // Step 2: Relax all edges |V| - 1 times. A simple shortest path from src
    // to any other vertex can have at-most |V| - 1 edges
    for (int i = 1; i <= V-1; i++)
    {
        for (int j = 0; j < E; j++)
        {
            int u = graph->edge[j].src;
            int v = graph->edge[j].dest;
            int weight = graph->edge[j].weight;
            if (dist[u] + weight < dist[v])
                dist[v] = dist[u] + weight;
        }
    }

    // Step 3: check for negative-weight cycles.  The above step guarantees
    // shortest distances if graph doesn't contain negative weight cycle.
    // If we get a shorter path, then there is a cycle.
    for (int i = 0; i < E; i++)
    {
        int u = graph->edge[i].src;
        int v = graph->edge[i].dest;
        int weight = graph->edge[i].weight;
        if (dist[u] + weight < dist[v])
            printf("Graph contains negative weight cycle");
    }

    printArr(dist, V);

    return;
}

void CurrencyArbExample()
{
 /* Let us create the graph given in above example */
int V = 3;  // Number of vertices in graph
int E = 6;  // Number of edges in graph
struct Graph* graph = createGraph(V, E);

enum Nodes
{
  USD,
  EUR,
  GBP
};

double USDEUR = .8;
double EURGBP = .8;
double GBPUSD = 1.7;

double logUSDEUR = log(USDEUR);
double logEURGBP = log(EURGBP);
double logGBPUSD = log(GBPUSD);

double logOneOverUSDEUR = log(1.0 / USDEUR);
double logOneOverEURGBP = log(1.0 / EURGBP);
double logOneOverGBPUSD = log(1.0 / GBPUSD);

std::cout << logUSDEUR << " " << logEURGBP << " " << logGBPUSD << " "
          << logOneOverUSDEUR << " " << logOneOverEURGBP << " " << logOneOverGBPUSD
          << std::endl;

//("usd", "euro") : log(1/usd_euro),
//("euro", "gbp") : log(1/euro_gbp),
//("gbp", "usd") : log(1/gbp_usd),
//("euro", "usd") : log(usd_euro),
//("gbp", "euro") : log(euro_gbp),
//("usd", "gbp") : log(gbp_usd)

graph->edge[0].src = USD;
graph->edge[0].dest = EUR;
graph->edge[0].weight = logOneOverUSDEUR;

graph->edge[1].src = EUR;
graph->edge[1].dest = GBP;
graph->edge[1].weight = logOneOverEURGBP;

graph->edge[2].src = GBP;
graph->edge[2].dest = USD;
graph->edge[2].weight = logOneOverGBPUSD;

graph->edge[3].src = EUR;
graph->edge[3].dest = USD;
graph->edge[3].weight = logUSDEUR;

graph->edge[4].src = GBP;
graph->edge[4].dest = EUR;
graph->edge[4].weight = logEURGBP;

graph->edge[5].src = USD;
graph->edge[5].dest = GBP;
graph->edge[5].weight = logGBPUSD;


BellmanFord(graph, 0);
}


// Driver program to test above functions
int main()
{
    CurrencyArbExample();

    return 0;
}

Answer 1

your weight attribute of struct Edge is of type int while you assign double values, namely the computed logarithms of exchanges rates.

turn on the warnings of your compiler, you should receive type narrowing warnings (at least).