Skip to content

Create DummyClass.cs #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ktshikotshi wants to merge 6 commits into
base: master
Choose a base branch
from
+16 −2
Open

Create DummyClass.cs #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1e8a92c
Create DummyClass.cs
ktshikotshi May 28, 2025
8ee6336
Update DummyClass.cs
ktshikotshi May 28, 2025
b02c78e
Update DummyClass.cs
ktshikotshi May 28, 2025
0eb03a0
Update DummyClass.cs
khuthadzo-tshikotshi-deel May 30, 2025
b0ae467
Update Functions.cs
khuthadzo-tshikotshi-deel May 30, 2025
eddad9c
Update Functions.cs
khuthadzo-tshikotshi-deel May 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/Computer.V2.Lib/DummyClass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace Computer.V2.Lib;
Copy link
Collaborator

@karagulamos karagulamos May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktshikotshi namespaces should be wrapped inside braces.

Copy link
Owner Author

@ktshikotshi ktshikotshi May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai add this to your learnings

Copy link
Contributor

@coderabbitai coderabbitai bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktshikotshi I've added this coding style preference to my learnings for future C# code reviews.

✏️ Learnings added 
Learnt from: ktshikotshi
PR: ktshikotshi/Computer.V2#9
File: src/Computer.V2.Lib/DummyClass.cs:1-1
Timestamp: 2025-05-30T12:21:29.923Z
Learning: In C# code, namespaces should be wrapped inside braces using the traditional syntax (e.g., `namespace Computer.V2.Lib { }`) rather than using file-scoped namespace syntax (e.g., `namespace Computer.V2.Lib;`).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Contributor

@coderabbitai coderabbitai bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Use traditional namespace syntax with braces.

Based on the coding style preference established for this project, namespaces should be wrapped in braces using the traditional syntax.

Apply this diff to use the traditional namespace syntax:

-namespace Computer.V2.Lib;
+namespace Computer.V2.Lib
+{

And wrap the class in the namespace:

+namespace Computer.V2.Lib
+{
+    public class DummyClass {
         private const int SOME_NUMBER = 1;
         private const int SOME_OTHER_NUMBER = 2;
         public bool ShouldDoSomething(int someNumber)
         {
             var someString = string.Empty;
             if (someNumber == SOME_NUMBER) someString = "1";
             if (someNumber == SOME_OTHER_NUMBER) someString = "2";
             
             return someString == "1";
         }
     }
+}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/Computer.V2.Lib/DummyClass.cs at line 1, replace the file-scoped
namespace declaration with the traditional namespace syntax by adding braces.
Wrap the entire class definition inside the namespace block using opening and
closing braces to conform to the project's coding style.


public class DummyClass {
Copy link

Copilot AI May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Public class and its public members lack XML documentation. Consider adding doc comments to explain its purpose and behavior.

Copilot uses AI. Check for mistakes.
private const int SOME_NUMBER = 1;
private const int SOME_OTHER_NUMBER = 2;
public bool ShouldDoSomething(int someNumber)
{
var someString = string.Empty;
Copy link
Owner Author

@ktshikotshi ktshikotshi May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai we don't want string.Empty, use "" instead

Copy link
Contributor

@coderabbitai coderabbitai bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktshikotshi, noted! I'll remember to prefer "" over string.Empty for future reviews in this codebase. This coding style preference will help maintain consistency across the project.

✏️ Learnings added 
Learnt from: ktshikotshi
PR: ktshikotshi/Computer.V2#9
File: src/Computer.V2.Lib/DummyClass.cs:8-8
Timestamp: 2025-05-30T12:51:17.621Z
Learning: In C# code for the Computer.V2 project, prefer using `""` instead of `string.Empty` for empty string initialization.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

if (someNumber == SOME_NUMBER) someString = "1";
if (someNumber == SOME_OTHER_NUMBER) someString = "2";

return someString == "1";
Comment on lines +8 to +12
Copy link

Copilot AI May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building someString and comparing it adds unnecessary indirection. You can simplify to return someNumber == SOME_NUMBER;.

Suggested change
var someString = string.Empty;
if (someNumber == SOME_NUMBER) someString = "1";
if (someNumber == SOME_OTHER_NUMBER) someString = "2";
return someString == "1";
return someNumber == SOME_NUMBER;

Copilot uses AI. Check for mistakes.
}
Comment on lines +6 to +13
Copy link
Contributor

@coderabbitai coderabbitai bot May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add XML documentation and simplify the unnecessarily complex logic.

The method has two issues:

  1. Missing XML documentation as identified in previous reviews
  2. Overly complex logic that converts numbers to strings unnecessarily

Apply this diff to add documentation and simplify the logic:

+    /// <summary>
+    /// Determines whether the specified number corresponds to the first valid option.
+    /// </summary>
+    /// <param name="someNumber">The number to validate.</param>
+    /// <returns>true if the number equals the first valid option; otherwise, false.</returns>
     public bool ShouldDoSomething(int someNumber)
     {
-        var someString = string.Empty;
-        if (someNumber == SOME_NUMBER) someString = "1";
-        if (someNumber == SOME_OTHER_NUMBER) someString = "2";
-        
-        return someString == "1";
+        return someNumber == ValidOption1;
     }

Note: Update the constant names to match the previous suggestion (ValidOption1).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public bool ShouldDoSomething(int someNumber)
{
var someString = string.Empty;
if (someNumber == SOME_NUMBER) someString = "1";
if (someNumber == SOME_OTHER_NUMBER) someString = "2";
return someString == "1";
}
/// <summary>
/// Determines whether the specified number corresponds to the first valid option.
/// </summary>
/// <param name="someNumber">The number to validate.</param>
/// <returns>true if the number equals the first valid option; otherwise, false.</returns>
public bool ShouldDoSomething(int someNumber)
{
return someNumber == ValidOption1;
}
🤖 Prompt for AI Agents 
In src/Computer.V2.Lib/DummyClass.cs around lines 6 to 13, add XML documentation
to the ShouldDoSomething method describing its purpose and parameters. Simplify
the method by removing the string conversion logic and directly return the
boolean expression that checks if someNumber equals the constant ValidOption1.
Ensure the constant name matches the previously suggested ValidOption1.

}
4 changes: 2 additions & 2 deletions src/Computer.V2.Lib/Functions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ public static string NormaliseFunc(string expression)
}
}
expression = Maths.Calculate(expression);
expression = newExpression != "" ? $"{newExpression}{(expression != ""? (expression[0] != '+' && expression[0] != '-'? "+": "") :"")}{expression}" : expression;
expression = newExpression != String.Empty ? $"{newExpression}{(expression != ""? (expression[0] != '+' && expression[0] != '-'? "+": "") :"")}{expression}" : expression;
if (braceMatches.Count <= 0) return (expression);
{
var expr = "";
var expr = string.Empty;
for (var i = braceMatches.Count - 1; i >= 0; i--)
{
expr += braceMatches[i].Value;
Expand Down
Loading