0

I have following code snippet that will append filename with current timestamp and it's working fine. Just want to make sure this is best way to append strings in c# 10, if not then how we can we make below code more efficient? ex: testfile.txt ->o/p testfile_timestamp.txt

 string[] strName = formFile.FileName.Split('.');
                string updatedFilename = strName[0] + "_"
                   + DateTime.Now.ToUniversalTime().ToString("THHmmssfff") + "." +
                   strName[strName.Length - 1];
Timothy G.
  • 6,335
  • 7
  • 30
  • 46
ks003
  • 3
  • 2
  • You can have a look at `StringBuilder`: https://learn.microsoft.com/en-us/dotnet/api/system.text.stringbuilder?view=net-7.0. However - in this simple case I doubt it will deliver a significant improvement. As always in case of performance it's important to measure and compare. – wohlstad Jan 10 '23 at 12:28
  • 2
    https://stackoverflow.com/questions/21078/most-efficient-way-to-concatenate-strings?rq=1 – Denis Schaf Jan 10 '23 at 12:33
  • 1
    Define "efficient" ? – Fildor Jan 10 '23 at 12:35
  • @wohlstad in this particular case it will deliver a performance degradation. https://stackoverflow.com/a/21131/9945524 – Mark Cilia Vincenti Jan 10 '23 at 12:49
  • @MarkCiliaVincenti see my first comment about the need to measure, and my doubt that it will actually boost performance in this case. – wohlstad Jan 10 '23 at 12:50
  • @wohlstad I replied to you specifically because I read your comment. You had earlier written that you "doubt it will deliver a significant improvement" which sounds very much like you're implying an expectation of _some_ improvement, just not significant. It's certainly not the case. Without available benchmarks, the right assumption would be that it will lead to a small performance degradation. – Mark Cilia Vincenti Jan 10 '23 at 12:56
  • @MarkCiliaVincenti I certainly agree a benchmark is a must, and since I don't have specific experince benchmarking string concatenation I accept your intuition about the degradation. – wohlstad Jan 10 '23 at 12:59
  • 1
    I added a benchmark to my answer ... – Fildor Jan 10 '23 at 13:21

2 Answers2

1

How about this:

// Just a stupid method name for demo, you'll find a better one :)
public static string DatifyFileName(string fileName)
{
// Use well tested Framework method to get filename without extension
    var nameWithoutExtension = System.IO.Path.GetFileNameWithoutExtension(fileName);
// Use well tested Framework method to get extension
    var extension = System.IO.Path.GetExtension(fileName);
// interpolate to get the desired output.
    return $"{nameWithoutExtension}_{DateTime.Now.ToUniversalTime().ToString("THHmmssfff")}{extension}";
}

Or if you are familiar with Span<char>:

public static string DatifyFileName(ReadOnlySpan<char> fileName)
{
    var lastDotIndex = fileName.LastIndexOf('.');
    //Maybe : if( lastDotIndex < 0 ) throw ArgumentException("no extension found");
    var nameWithoutExtension = fileName[..lastDotIndex];
    var extension = fileName[lastDotIndex..];
    return $"{nameWithoutExtension}_{DateTime.Now.ToUniversalTime().ToString("THHmmssfff")}{extension}";
}

Fiddle


And just to give some fire to the discussion :D ...

BenchmarkDotNet=v0.13.3, OS=Windows 10 (10.0.19044.2364/21H2/November2021Update)
Intel Core i9-10885H CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK=7.0.101
  [Host]     : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2
  DefaultJob : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2


|           Method |       Mean |    Error |   StdDev | Ratio | RatioSD |   Gen0 | Allocated | Alloc Ratio |
|----------------- |-----------:|---------:|---------:|------:|--------:|-------:|----------:|------------:|
|     Interpolated |   906.7 ns | 16.92 ns | 16.61 ns |  1.08 |    0.02 | 0.0458 |     384 B |        1.66 |
| InterpolatedSpan |   842.0 ns | 13.06 ns | 12.22 ns |  1.00 |    0.00 | 0.0277 |     232 B |        1.00 |
|    StringBuilder | 1,010.8 ns |  6.70 ns |  5.94 ns |  1.20 |    0.02 | 0.1068 |     904 B |        3.90 |
|         Original |   960.0 ns | 18.68 ns | 19.19 ns |  1.14 |    0.03 | 0.0734 |     616 B |        2.66 |

// * Hints *
Outliers
  Benchmark.StringBuilder: Default -> 1 outlier  was  removed (1.03 us)
  Benchmark.Original: Default      -> 2 outliers were removed (1.03 us, 1.06 us)

// * Legends *
  Mean        : Arithmetic mean of all measurements
  Error       : Half of 99.9% confidence interval
  StdDev      : Standard deviation of all measurements
  Ratio       : Mean of the ratio distribution ([Current]/[Baseline])
  RatioSD     : Standard deviation of the ratio distribution ([Current]/[Baseline])
  Gen0        : GC Generation 0 collects per 1000 operations
  Allocated   : Allocated memory per single operation (managed only, inclusive, 1KB = 1024B)
  Alloc Ratio : Allocated memory ratio distribution ([Current]/[Baseline])
  1 ns        : 1 Nanosecond (0.000000001 sec)
Fildor
  • 14,510
  • 4
  • 35
  • 67
  • This is neater and more robust code, and won't choke on .tar.gz files for example, but I'm not convinced it is more _efficient_. – Mark Cilia Vincenti Jan 10 '23 at 12:42
  • Nobody has _defined_ what "efficient" is supposed to mean in this context. @MarkCiliaVincenti – Fildor Jan 10 '23 at 12:44
  • You're right there, perhaps I wrongly assumed it to mean faster and with lower memory use. – Mark Cilia Vincenti Jan 10 '23 at 12:45
  • Would have to write a benchmark to prove that. At least this is readable, but in fact does have a bug with ".tar.gz" because the `Path` methods will only consider ".gz" the extension ... so this has potential for improvements. – Fildor Jan 10 '23 at 12:49
  • The original, though would make "test.tar.gz" into "test_T135345667.gz" ... so, there would be some clarification on the requirements needed, anyways. – Fildor Jan 10 '23 at 12:53
  • Yes, we'd need to write a benchmark to prove that but it's a good educated guess because while the original code splits the string once and uses that operation to get both parts of the filename, this code you provided will split the string twice. – Mark Cilia Vincenti Jan 10 '23 at 12:53
  • Correct. Now, if we want to milk the very last bit out of this, I'd tend to switch to a solution based on `Span`... – Fildor Jan 10 '23 at 12:54
  • It's strange that .NET doesn't have anything out of the box for parsing files/paths and providing all parts, computed once. – Mark Cilia Vincenti Jan 10 '23 at 13:01
  • Well, the building blocks are there. Haven't missed something like that, tbh. – Fildor Jan 10 '23 at 13:05
  • I am not 100% sure, but afraid not. – Fildor Jan 10 '23 at 13:10
0

Strings are immutable. whenever you modify any string, it is recreated in the memory. Hence to overcome this you should always use StringBuilder. Sample code shown below:

string[] strName = formFile.FileName.Split('.');
StringBuilder updatedFilename  = new StringBuilder();
updatedFilename.Append(strName[0]);
updatedFilename.Append("_");
updatedFilename.Append(DateTime.Now.ToUniversalTime().ToString("THHmmssfff"));
updatedFilename.Append(".");
updatedFilename.Append(strName[strName.Length - 1]);

// You can get this using `ToString()` method
string filename = updatedFilename.ToString();
HarrY
  • 607
  • 4
  • 14
  • I am not convinced that in this particular case, `StringBuilder` will give a performance benefit. In fact, I'd suspect that if you were to benchmark it, you'd see it performs inferiorly. You shouldn't "always use StringBuilder" or you'd be adding unnecessary overhead sometimes. You should use StringBuilder wisely, when it makes sense to do so (eg in loops that append to a string). – Mark Cilia Vincenti Jan 10 '23 at 12:40
  • @MarkCiliaVincenti He did not ask about performance benefits. He asked how to append string. Better read description carefully. – HarrY Jan 10 '23 at 12:44
  • so what do you think was meant by 'Efficient'? – Mark Cilia Vincenti Jan 10 '23 at 12:46