fluffynuts commented on code in PR #203:
URL: https://github.com/apache/logging-log4net/pull/203#discussion_r1825457681
##########
src/log4net.Tests/Appender/RollingFileAppenderTest.cs:
##########
@@ -524,23 +523,10 @@ private static string GetCurrentFile() =>
/// <param name="sBackupGroup"></param>
/// <param name="iBackupFileLength"></param>
/// <returns></returns>
- private static List<RollFileEntry>
MakeBackupFileEntriesFromBackupGroup(string sBackupGroup, int iBackupFileLength)
- {
- string[] sFiles = sBackupGroup.Split(' ');
-
- List<RollFileEntry> result = [];
-
- for (int i = 0; i < sFiles.Length; i++)
- {
- // Weed out any whitespace entries from the array
- if (sFiles[i].Trim().Length > 0)
- {
- result.Add(new RollFileEntry(sFiles[i], iBackupFileLength));
- }
- }
-
- return result;
- }
+ private static List<RollFileEntry>
MakeBackupFileEntriesFromBackupGroup(string sBackupGroup, int
iBackupFileLength)
+ => sBackupGroup.Split(' ').Where(file => file.Trim().Length > 0)
Review Comment:
personal note: i really don't like inline method bodies - it's like I take a
little longer to grok them over, eg
```csharp
private static List<RollFileEntry> MakeBackupFileEntriesFromBackupGroup(
string sBackupGroup,
int iBackupFileLength
)
{
return sBackupGroup.Split(' ')
.Where(file => file.Trim().Length > 0)
.Select(file => new RollFileEntry(file, iBackupFileLength))
.ToList()
}
```
Perhaps it's just me though...
I'd also like to make two "soft" suggestions for code style going forward:
1. vertical parameters (as in my example above) - see
https://youtu.be/1WL-SudgJAo (I had to re-upload this because it had been
removed, so I got it from archive.org and re-upped)
2. dropping type prefixes for variables / parameters (eg _i_BackupFileLength
and _s_BackupGroup)
I wouldn't go on a major refactoring spree, just fix things as I'm in the
area.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
