gtorrent / gtorrent-core

Core library of gTorrent which handles everything but UI/UX
GNU General Public License v3.0
39 stars 12 forks source link

Rewrote getFileStringSize #41

Closed charles-ramsey closed 10 years ago

charles-ramsey commented 10 years ago

-Function now implements binary search instead of that if-clause mess. -Constants are now defined for 1 KB, 1 MB, 1 GB instead of repeated 1024 multiplication/division. -String contains a double formatted to 1 decimal place; the previous code attempted to print 3 decimal places but used integer division. -Removed the file_size <= 0 check. We should print "0 B" instead of "", and negative file_sizes should throw an error elsewhere.

benwaffle commented 10 years ago

consider #34

charles-ramsey commented 10 years ago

Sorry, I didn't see that. I'm new to git. Anyway, I think the log approach is cleaner, though the pow(1024,offset) call is unnecessary as the possible values [1,1024,1024^2,1024^3] could easily be precomputed and accessed like the suffix array.

I just did some benchmarking with generated normally-distributed data and while my code is faster, the difference isn't that significant. 1M iterations took 1.2s vs. 1.6s for the log version using the -O2 flag and g++.

benwaffle commented 10 years ago

I guess then it doesn't matter. We should choose the one that is easy to understand and is concise.

benwaffle commented 10 years ago

yet another suggestion

void scaledata(double prefix, double in, double *data, const char **unitp) {
    static int i;
    static const char units[2][8][4] = {
        { "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB" },
        { "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB" }
    };
    *data = in;
    for (i = 0; *data > prefix && i < 7; i++)
        *data /= prefix;
    *unitp = units[prefix==1024.0][i];
}

scaledata(1024, 2048, &outamns, &unitpointer);
outamnt = 2
outpointer = MiB

scaledata(1000, 2000, &outamns, &unitpointer);
outamnt = 2
outpointer = MB
charles-ramsey commented 10 years ago

That's equivalent to the logarithm method, only more difficult to read.

fuyukaidesu commented 10 years ago

We are merging this one or #44 ?