Okay, this is going to be a big post. Hope you like feedback. I'll preface this with saying that this analysis is from a skim read of your project, and is my no means an in-depth review. (Judging by your code, I'd say with a degree of certainty, you were following this tutorial)
Key notes:
- You have a single js file that's almost 600 lines and is running in a err... linear fashion. Consider moving your events to their own files and then having a single startup file. Use module.exports or export default function for the events and then move the other functions to an independent file.
- You are requiring fs every time you use it. This is a bad practice.
- You are importing winston for logging, which isn't a bad idea, but you declare it, use it once, then revert to using console.log and console.error throughout the rest of your code.
- You have fs listed in your package.json - don't do this. It's been part of Node since 8.0 iirc.
- You are storing everything inside of json files and writing to them constantly. This is a terrible idea, and there are numerious articles on the internet on why you shouldn't use json for persistence (config files are fine, obviously).
- We are living in 2019 but your code is acting like it's 2012. Use modern syntax. E.g. replace var with let/const, use arrow functions, etc.
- You are using extremely nested if functions, inside of loops. Use break, continue, and return statements for better control.
- You are chaining a lot of "if {} else if {} else if {} else if {}" etc. I would suggest using switch case statements instead.
- You have unused variables declared throughout your code.
- You are sending messages to Discord using a deprecated method (sendMessage had support removed like two versions ago). Use msg.channel.send instead.
- Your code is seriously lacking formatting in many places. Malformed indents (sometimes spaces are used), missing indents at times, random line breaks throughout your code, etc.
- Your message event function is an absolute disaster. With all due respect, you need to seriously rework that. Using a switch and case as a command handler is a pretty daft way of command checking; look into using module.exports/export default function. In addition you are nested function declarations, which is one of the most dated JS practices I've ever seen. Don't do that unless you really have to. Your message event is pulling in unneeded params, you only need the msg object.
- From what I can tell you are hardlocking the prefix to ! which is a very common prefix by other bots and will cause a lot of issues in other servers. If you use a common prefix, make a command to change it.
- Loading the base page you are doing an "if (err) throw err;". If the forums went down for even a second, this would crash your bot. Use proper error handling.
- COMMENT YOUR CODE. When it's got so many formatting issues, it's very difficult to analyse.
- Reading this (and I could be wrong with this assumption), I am under the impression you have copied and pasted a lot of different snippets from various places.
- The end of your message event has a duplicate switch case that is testing against true. I can't even... what?
I would really suggest looking into async/await rather than simply relying on your setTimeout. Using setTimeout over and over (especially with your get requests and extremely long loops) can cause a lot of undesired behaviour and exceptions to be thrown. You are effectively going "yeah, this function will probably take about... err five seconds? I guess?".
(04-16-2019, 04:37 PM)dr lameos Wrote: Price:
This bot is free, but if you'd like to donate you can send credits to:
Lameos.Bank1
If you're feeling especially generous you can send bitcoin to:
16FppmC7Kbs3BQm3ePVyYErfi8nB8yAJLw (other crypto is also gratefully accepted - PM me if you want)
I'll end off by saying I personally find it extremely out of taste to request donations for a community project. I, like many of the others before me, have invested hundreds of hours into the development of this game and utilities for it and have never asked a penny for it. If this project took you months, it'd be a little more understandable, but something this small that is so evidently from an amateur just rubs me the wrong way.
I hope my notes are helpful to you, and if you have questions post here or message me over Discord. As many of the community know, I am always eager to help others learn code.
