Skip to content

Conversation

@Founntain
Copy link

@Founntain Founntain commented Jul 27, 2022

When some floofs and I were talking about the FloofBot on the r/Furry Discord, we were talking about the weird and painfully looking codebase and that it definitely needed a refactor to the current standards, so:

I took the time to refactor the code base to current standards of C# and .NET

Things I changed:

  • Not using var where it should be used and on other places it was used
  • Not using Object Initializers where they can be used, on the other hand there were used on other places
  • Renamed Exception variables from e to ex. e is reserved for events; an Exception is not an event Apperently, a lot of people use e for exceptions and is also the default, when creating a try-block. Even tho that it is less verbose, I reverted that change
  • Changed some method signatures to async, because those methods were only called in async Task members, because of the change to async we can now also use the async overloads in those methods, making it completely async
  • Changed a lot of formatting, regarding empty lines after a bracket, double empty lines, unnessecary returns
  • Inverting a lot of if's to reduce nesting
  • Fixed naming for private properties, for local variables etc.
  • Fixed naming for Methods in classes. Methods always start with a capital letter, no matter if it is private or not. sorry java devs

What I noticed:

  • The FloofDataContext sometimes get's passed as an argument to classes and stays there as an global variable. Usually you don't do that and use a using statement instead, so that resourced get freed and the connection can be closed. That's the EF (Core) way of using a context. Opening it via (await) using(var floofDb = new FloofDataContext()) It did this on some places where I know it could be done, but at some places I were not sure. This is a bad approach, when you don't use Dependency Injection and passing it through constructors (which is bad habit).
  • The code base was really inconsistent, resulting in bad readability, using var and the not and then again is weird. Either use only var or don't.
  • The same goes for object initializers. They were used, 6 lines later not, then again and 6 lines later not. This is also weird. Either use it or don't.
  • Sometimes a blank line was missing between members, resulting also in bad readability.
  • I also saw some occurences where the async overloads where not used, even tho when you are in an async Method and can use the async overload without any issues
  • The current language level is set to C# 8.0 and not to latest. I would suggest to set it to latestMajor

I would greatly appreaciate if this will be looked through and tested by some people, esspecialy from the maintainer and people that know most of it's codebase and how it runs.
Because you people have more experience using this bot than I, for now.

@Founntain Founntain changed the title Refactored the code base to current C# and .NET Standards Refactored the codebase to current C# and .NET Standards Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant