Skip to content

Commit dd3494d

Browse files
fix: Misc code quality improvements (#871)
- Refactor path handling to use Path.Join for improved readabilitu
1 parent cd99966 commit dd3494d

File tree

11 files changed

+42
-58
lines changed

11 files changed

+42
-58
lines changed

EssentialCSharp.Chat.Shared/Services/AIChatService.cs

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -341,21 +341,17 @@ private static async Task<ResponseCreationOptions> CreateResponseOptionsAsync(
341341
string responseText = string.Empty;
342342
string responseId = response.Value.Id;
343343

344-
foreach (var outputItem in response.Value.OutputItems)
345-
{
346344
#pragma warning disable OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
347-
if (outputItem is MessageResponseItem messageItem &&
348-
messageItem.Role == MessageRole.Assistant)
349-
{
350-
var textContent = messageItem.Content?.FirstOrDefault()?.Text;
351-
if (!string.IsNullOrEmpty(textContent))
352-
{
353-
responseText = textContent;
354-
break;
355-
}
356-
}
357-
#pragma warning restore OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
345+
var assistantMessage = response.Value.OutputItems
346+
.OfType<MessageResponseItem>()
347+
.FirstOrDefault(m => m.Role == MessageRole.Assistant &&
348+
!string.IsNullOrEmpty(m.Content?.FirstOrDefault()?.Text));
349+
350+
if (assistantMessage is not null)
351+
{
352+
responseText = assistantMessage.Content?.FirstOrDefault()?.Text ?? string.Empty;
358353
}
354+
#pragma warning restore OPENAI001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
359355

360356
return (responseText, responseId);
361357
}

EssentialCSharp.Chat.Shared/Services/ChunkingResultExtensions.cs

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Security.Cryptography;
22
using System.Text;
3+
using System.Linq;
34
using EssentialCSharp.Chat.Common.Models;
45

56
namespace EssentialCSharp.Chat.Common.Services;
@@ -8,24 +9,24 @@ public static partial class ChunkingResultExtensions
89
{
910
public static List<BookContentChunk> ToBookContentChunks(this FileChunkingResult result)
1011
{
11-
var chunks = new List<BookContentChunk>();
1212
int? chapterNumber = ExtractChapterNumber(result.FileName);
1313

14-
foreach (var chunk in result.Chunks)
15-
{
16-
string chunkText = chunk;
17-
string contentHash = ComputeSha256Hash(chunkText);
18-
19-
chunks.Add(new BookContentChunk
14+
var chunks = result.Chunks
15+
.Select(chunkText =>
2016
{
21-
Id = Guid.NewGuid().ToString(),
22-
FileName = result.FileName,
23-
Heading = ExtractHeading(chunkText),
24-
ChunkText = chunkText,
25-
ChapterNumber = chapterNumber,
26-
ContentHash = contentHash
27-
});
28-
}
17+
var contentHash = ComputeSha256Hash(chunkText);
18+
return new BookContentChunk
19+
{
20+
Id = Guid.NewGuid().ToString(),
21+
FileName = result.FileName,
22+
Heading = ExtractHeading(chunkText),
23+
ChunkText = chunkText,
24+
ChapterNumber = chapterNumber,
25+
ContentHash = contentHash
26+
};
27+
})
28+
.ToList();
29+
2930
return chunks;
3031
}
3132

EssentialCSharp.Chat/Program.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ void WriteChunkingResult(FileChunkingResult result, TextWriter writer)
328328
outputDirectory.Create();
329329
foreach (var result in results)
330330
{
331-
var outputFile = Path.Combine(outputDirectory.FullName, Path.GetFileNameWithoutExtension(result.FileName) + ".chunks.txt");
331+
var outputFile = Path.Join(outputDirectory.FullName, Path.GetFileNameWithoutExtension(result.FileName) + ".chunks.txt");
332332
using var writer = new StreamWriter(outputFile, false);
333333
WriteChunkingResult(result, writer);
334334
Console.WriteLine($"Wrote: {outputFile}");

EssentialCSharp.Web.Tests/ListingSourceCodeServiceTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ private static ListingSourceCodeService CreateService()
133133
private static DirectoryInfo GetTestDataPath()
134134
{
135135
string baseDirectory = AppContext.BaseDirectory;
136-
string testDataPath = Path.Combine(baseDirectory, "TestData");
136+
string testDataPath = Path.Join(baseDirectory, "TestData");
137137

138138
DirectoryInfo testDataDirectory = new(testDataPath);
139139

EssentialCSharp.Web.Tests/WebApplicationFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ protected override void ConfigureWebHost(IWebHostBuilder builder)
5959
// Replace IListingSourceCodeService with one backed by TestData
6060
services.RemoveAll<IListingSourceCodeService>();
6161

62-
string testDataPath = Path.Combine(AppContext.BaseDirectory, "TestData");
62+
string testDataPath = Path.Join(AppContext.BaseDirectory, "TestData");
6363
var fileProvider = new PhysicalFileProvider(testDataPath);
6464
services.AddSingleton<IListingSourceCodeService>(sp =>
6565
{

EssentialCSharp.Web/Areas/Identity/Pages/Account/Logout.cshtml.cs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,8 @@ public async Task<IActionResult> OnPost(string? returnUrl = null)
1111
{
1212
await signInManager.SignOutAsync();
1313
logger.LogInformation("User logged out.");
14-
if (returnUrl is not null)
15-
{
16-
return LocalRedirect(returnUrl);
17-
}
18-
else
19-
{
2014
// This needs to be a redirect so that the browser performs a new
2115
// request and the identity for the user gets updated.
22-
return RedirectToPage();
23-
}
16+
return returnUrl is not null ? LocalRedirect(returnUrl) : RedirectToPage();
2417
}
2518
}

EssentialCSharp.Web/Controllers/HomeController.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public IActionResult Index()
2222
}
2323
else if (siteMapping is not null)
2424
{
25-
string filePath = Path.Combine(hostingEnvironment.ContentRootPath, Path.Combine(siteMapping.PagePath));
25+
string filePath = Path.Join(hostingEnvironment.ContentRootPath, Path.Join(siteMapping.PagePath));
2626
HtmlDocument doc = new();
2727
doc.Load(filePath);
2828
string headHtml = doc.DocumentNode.Element("html").Element("head").InnerHtml;
@@ -74,7 +74,7 @@ public IActionResult Home()
7474
public IActionResult Guidelines()
7575
{
7676
ViewBag.PageTitle = "Coding Guidelines";
77-
FileInfo fileInfo = new(Path.Combine(hostingEnvironment.ContentRootPath, "Guidelines", "guidelines.json"));
77+
FileInfo fileInfo = new(Path.Join(hostingEnvironment.ContentRootPath, "Guidelines", "guidelines.json"));
7878
if (!fileInfo.Exists)
7979
{
8080
return RedirectToAction(nameof(Error), new { errorMessage = "Guidelines could not be found", statusCode = 404 });

EssentialCSharp.Web/Extensions/SiteMappingListExtensions.cs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,9 @@ public static class SiteMappingListExtensions
1616
{
1717
return siteMappings.FirstOrDefault();
1818
}
19-
foreach (string? potentialMatch in key.GetPotentialMatches())
20-
{
21-
if (siteMappings.FirstOrDefault(x => x.Keys.Any(x => x == potentialMatch)) is { } siteMap)
22-
{
23-
return siteMap;
24-
}
25-
}
26-
return null;
19+
return key.GetPotentialMatches()
20+
.Select(potentialMatch => siteMappings.FirstOrDefault(x => x.Keys.Any(k => k == potentialMatch)))
21+
.FirstOrDefault(siteMap => siteMap != null);
2722
}
2823
/// <summary>
2924
/// Finds percent complete based on a key.

EssentialCSharp.Web/Helpers/SitemapXmlHelpers.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public static void GenerateAndSerializeSitemapXml(DirectoryInfo wwwrootDirectory
2323
{
2424
GenerateSitemapXml(wwwrootDirectory, siteMappings, routeConfigurationService, baseUrl, out List<SitemapNode> nodes);
2525
XmlSerializer sitemapProvider = new();
26-
var xmlPath = Path.Combine(wwwrootDirectory.FullName, "sitemap.xml");
26+
var xmlPath = Path.Join(wwwrootDirectory.FullName, "sitemap.xml");
2727
sitemapProvider.Serialize(new SitemapModel(nodes), xmlPath, true);
2828
logger.LogInformation("sitemap.xml successfully written to {XmlPath}", xmlPath);
2929
}

EssentialCSharp.Web/Services/RouteConfigurationService.cs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,13 @@ private HashSet<string> ExtractStaticRoutes()
4747

4848
// For actions without attribute routes, use conventional routing
4949
if (actionDescriptor.AttributeRouteInfo?.Template == null &&
50-
actionDescriptor.RouteValues.TryGetValue("action", out var actionName) &&
51-
actionDescriptor.RouteValues.TryGetValue("controller", out var controllerName))
50+
actionDescriptor.RouteValues.TryGetValue("action", out var actionName) &&
51+
actionDescriptor.RouteValues.TryGetValue("controller", out var controllerName) &&
52+
controllerName?.Equals("Home", StringComparison.OrdinalIgnoreCase) == true &&
53+
actionName != null)
5254
{
53-
if (controllerName?.Equals("Home", StringComparison.OrdinalIgnoreCase) == true && actionName != null)
54-
{
55-
// Use the action name directly as the route
56-
routes.Add(actionName.ToLowerInvariant());
57-
}
55+
// Use the action name directly as the route
56+
routes.Add(actionName.ToLowerInvariant());
5857
}
5958
}
6059

0 commit comments

Comments
 (0)