Assigning references values within string.Format()

Question

I have a method which takes in a reference double and returns a string, while also modifying the value of the reference:

const long OneKb = 1024;
const long OneMb = OneKb * 1024;
const long OneGb = OneMb * 1024;
const long OneTb = OneGb * 1024;

public string GetLargestDataSizeAndUnitOfMeasurement(ref double value, int decimalPlaces = 0)
{
    var asTb = Math.Round(value / OneTb, decimalPlaces);
    var asGb = Math.Round(value / OneGb, decimalPlaces);
    var asMb = Math.Round(value / OneMb, decimalPlaces);
    var asKb = Math.Round(value / OneKb, decimalPlaces);
    string unit = asTb > 1 ? string.Format("Tb", value = asTb)
    : asGb > 1 ? string.Format("Gb", value = asGb)
    : asMb > 1 ? string.Format("Mb", value = asMb)
    : asKb > 1 ? string.Format("Kb", value = asKb)
    : string.Format("B", value = Math.Round(value, decimalPlaces));
    return unit;
}

My question is whether it's acceptable to assign the reference a new value within the string.Format(), despite the value not being pertinent to that method itself. I could execute the if separately to modify value if I wanted to avoid this, but this seems cleaner and potentially more efficient at scale.

Answer 1

I really don't see anything wrong with doing assignment calls within a string.format method. But by looking at the given code, it could be avoided.

If you looking for cleaner code you could use an enum to store your data unit names and you could reduce the amount of variables you declaring. This way your code is more maintainable and if ever you want to use a higher data size unit you could just add it to enum list.

    public class DataSizeFormatter
    {
        const int OneKB = 1024;

        private enum DataSizes
        {
            B,
            KB,
            MB,
            GB,
            TB
        }

        public string GetLargestDataSizeAndUnitOfMeasurement(ref double value, int decimalPlaces = 0)
        {
            var highestExponent = (int)(Math.Log(value, OneKB));        // Get the highest power which you could assign to 1024 that would not be greater than the given value.
            var lengthOfDataSizeEnum = Enum.GetNames(typeof(DataSizes)).Length;     //Get the length of the enum list

            int highestExponentWithLimit = highestExponent < lengthOfDataSizeEnum ? highestExponent : lengthOfDataSizeEnum - 1;   //If the given value could be divided by a higher data unit than in your enum list then only use your highest data size unit.

            value = Math.Round(value / Math.Pow(OneKB, highestExponentWithLimit), decimalPlaces);   //round of your given value to the approriate data size.

            return ((DataSizes)highestExponentWithLimit).ToString();    //return the data size that was used to round of your given value.

        }
}

UPDATE:

Have a look at these similar questions which explains why it's fine to do assignments within parameters: Variable assignment within method parameter and Why do assignment statements return a value?.

UPDATE 2:

The question around a best approach to converting bytes to higher data storage units has been answered here: Does .NET provide an easy way convert bytes to KB, MB, GB, etc.?.

Answer 2

While having an assignment, value = asTb, inside of a String.Format() is valid. In terms of readability and way in which String.Format() is being used is unexpected behaviour.

MSDN docs states the reason for using String.Format() as:

Use String.Format if you need to insert the value of an object, variable, or expression into another string

Base on your code snippet this is not your intention. You simply want to return a formatted file size and its corresponding symbol.

public class FileSizeConverter
{
    public enum FileSizeSymbol
    {
        B,
        KB,
        MB,
        GB,
        TB
    }

    public string FormatByteSize(ref double fileSize, int decimalPlaces = 0)
    {
        var unit = FileSizeSymbol.B;
        while (fileSize >= 1024 && unit < FileSizeSymbol.TB)
        {
            fileSize = fileSize / 1024;
            unit++;
        }

        fileSize = Math.Round(fileSize, decimalPlaces, MidpointRounding.AwayFromZero);

        return unit.ToString();
    }
}

Answer 3

Create a small class which has Value and Unit properties, a ToString method and a constructor which takes a double as a parameter. Maybe call it DimensionedNumber.

Now you just create a DimensionedNumber by passing value to the constructor. If you want to display the DimensionedNumber you can just use the ToString() method.

Something like

public class DimensionedNumber
{
    public double Value{get; private set;}
    public string Dimension {get; private set;}

    const double OneKb = 1024.0;
    const double OneMb = OneKb * OneKb;
    const double OneGb = OneMb * OneKb;
    const double OneTb = OneGb * OneKb;

    public DimensionedNumber(double value)
    {
        if (value > OneTb) {
            Value = value / OneTb;
            Dimension = "Tb";
        } else if (value > OneGb) {
            Value = value / OneGb;
            Dimension = "Gb";
        } else if (value > OneMb) {
            Value = value / OneMb;
            Dimension = "Mb";
        } else if (value > OneKb) {
            Value = value / OneKb;
            Dimension = "Kb";
        } else {
            Value = value;
            Dimension = "";
        }
    }

    public string ToString(int decimalPlaces)
    {
        return Value.ToString("N" + decimalPlaces.ToString()) + Dimension;
    }
}

You would use it with

var displayValue = new DimenesionedNumber(12345678.9);
Console.WriteLine(displayValue.ToString(3));  // Three decimal places