Added more TODOs for all reviewed code, fixed some minor things, but …

…giving up on this mess now, I would prefer a rewrite in a language I am more comfortable in

src/main/java/com/github/nija123098/tipbot/Database.java

@@ -16,10 +16,15 @@
 import java.util.Set;
 
 public class Database {
+    //TODO: this should not be in a database, only the blockchain contains the truth, the dash client is all we need to query this!
     public static final String BALANCES_TABLE = "balances";
+    //TODO: again, this is only needed for non registered users, otherwise we could send tips directly, this is just cumbersome
     public static final String RECEIVED_TABLE = "received";
+    //TODO: not using db properly (user should have this as a property)
     public static final String RECEIVING_ADDRESSES_TABLE = "receiving_addresses";
+    //TODO: unnecessary imo
     public static final String PREFERRED_CURRENCY = "preferred_currency";
+    //TODO: not sure why this has to be in a tip bot
     public static final String ANNOUNCEMENT_CHANNEL = "announcement_channel";
     private static final Connection CONNECTION;
     private static final QueryRunner RUNNER;
@@ -42,6 +47,7 @@ public class Database {
             dataSource.setUseUnicode(true);
             c = dataSource.getConnection();
         } catch (SQLException e) {
+            //TODO: Code issue: Avoid throwing raw exception types.
             throw new RuntimeException("Could not init database connection!", e);
         }
         CONNECTION = c;
@@ -61,20 +67,23 @@ private static <E> E select(String sql, ResultSetHandler<E> handler) {
         try{return RUNNER.query(sql, handler);
         } catch (SQLException e) {
             //TODO: all this is a very bad way of error handling
+            //TODO: Code issue: Avoid throwing raw exception types.
             throw new RuntimeException("Could not select: ERROR: " + e.getErrorCode() + " " + sql, e);
         }
     }
 
     private static void query(String sql) {
         try{RUNNER.update(sql);
         } catch (SQLException e) {
+            //TODO: Code issue: Avoid throwing raw exception types.
             throw new RuntimeException("Could not query: ERROR: " + e.getErrorCode() + " " + sql, e);
         }
     }
 
     private static void insert(String sql) {// set
         try{RUNNER.update(sql);
         } catch (SQLException e) {
+            //TODO: Code issue: Avoid throwing raw exception types.
             throw new RuntimeException("Could not insert: ERROR: " + e.getErrorCode() + " " + sql, e);
         }
     }
@@ -83,7 +92,9 @@ private static String quote(String id){
         return Character.isDigit(id.charAt(0)) ? id : "'" + id + "'";
     }
 
+    //TODO: Code issue: Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.
     private static final Set<String> EXISTING_TABLES = new HashSet<>();
+    //TODO: each db table is just a key value pair, using mysql for that is kinda wasteful and not really necessary
     private static void ensureTableExistence(String table){
         if (!EXISTING_TABLES.add(table)) return;
         try (ResultSet rs = CONNECTION.getMetaData().getTables(null, null, table, null)) {
@@ -93,6 +104,7 @@ private static void ensureTableExistence(String table){
             }// make
             Database.query("CREATE TABLE `" + table + "` (id TINYTEXT, value TINYTEXT)");
         } catch (SQLException e) {
+            //TODO: Code issue: Avoid throwing raw exception types.
             throw new RuntimeException("Could not ensure table existence", e);
         }
     }
@@ -107,6 +119,7 @@ public static void setValue(String table, IDiscordObject user, String value) thr
             } catch (SQLException e) {
                 Database.insert("INSERT INTO " + table + " (`id`, `value`) VALUES ('" + user.getStringID() + "','" + value + "');");
             }
+            //TODO: bad way of error handling!
             return null;
         });
     }
@@ -116,12 +129,14 @@ public static void resetValue(String table, IDiscordObject object){
         Database.query("DELETE FROM " + table + " WHERE id = " + quote(object.getStringID()));
     }
 
+    //TODO: what is with the typo defaul?
     public static String getValue(String table, IDiscordObject user, String defaul){
         ensureTableExistence(table);
         return Database.select("SELECT * FROM " + table + " WHERE id = " + Database.quote(user.getStringID()), set -> {
             try{set.next();
                 return set.getString(2);
             } catch (SQLException e) {
+                //TODO: bad way of error handling, why is this here?
                 if (!e.getMessage().equals("Current position is after the last row")) throw new RuntimeException("Error while getting value", e);
                 return defaul;
             }

src/main/java/com/github/nija123098/tipbot/Main.java

@@ -12,6 +12,7 @@
 import sx.blah.discord.handle.impl.obj.ReactionEmoji;
 import sx.blah.discord.handle.obj.IUser;
 import sx.blah.discord.handle.obj.Permissions;
+//TODO: unused import
 import sx.blah.discord.util.Image;
 import sx.blah.discord.util.PermissionUtils;
 import sx.blah.discord.util.RequestBuffer;
@@ -30,6 +31,8 @@
 import java.util.concurrent.atomic.AtomicInteger;
 
 public class Main {
+    //TODO: before we start: this code is super hard to test, debugging is the only option after reading errors in the log
+    //TODO: also setup and first user experience is not very thought out, many strange quirks users won't understand at first (e.g. how to mention a user for a tip seems broken)
     //TODO: general problems: if something is not setup (config, db, dash-cli, etc.), it just crashes with NullReferenceException or things stay null and are not working!
     //TODO: tons of global variables, very unclean code
     public static final String OK_HAND = "👌", PONG = "🏓";
@@ -68,10 +71,15 @@ public class Main {
                 FULL_HELP_MAP.put(name, command.getFullHelp());
             }));
         } catch (Exception e){
+            //TODO: Code issue: Avoid throwing raw exception types.
             throw new RuntimeException(e);
         }
     }
+
+    //TODO: Code issue: Document empty method body
     public static void main(String[] args) {}
+
+    //TODO: code issue: The method handle() has an NPath complexity of 5760
     @EventSubscriber
     public static void handle(MessageReceivedEvent event){
         if (event.getAuthor().isBot()) return;
@@ -85,6 +93,7 @@ public static void handle(MessageReceivedEvent event){
             RequestBuffer.request(() -> event.getChannel().sendMessage("I need to be able to send reactions in this channel in order to operate."));
             return;
         }
+        //TODO: code issue: Use one line for each declaration, it enhances code readability.
         int length, newLength;
         do {
             length = content.length();
@@ -105,6 +114,7 @@ public static void handle(MessageReceivedEvent event){
             if (ret.equals(OK_HAND) || ret.equals(PONG)) RequestBuffer.request(() -> event.getMessage().addReaction(ReactionEmoji.of(ret)));
             else RequestBuffer.request(() -> event.getChannel().sendMessage(ret));
         } catch (Exception e){
+            //TODO: code issue: An instanceof check is being performed on the caught exception. Create a separate catch clause for this exception type.
             if (e instanceof InputException) event.getChannel().sendMessage(e.getMessage());
             else issueReport(event, e instanceof WrappingException ? (Exception) e.getCause() : e);
         }
@@ -125,8 +135,10 @@ public static void handle(ReactionAddEvent event){
 
     public static IUser getUserFromMention(String mention){
         if (!(mention.startsWith("<@") && mention.endsWith(">"))) return null;
+        //TODO: Code issue: Avoid reassigning parameters such as 'mention'
         mention = mention.replace("!", "");
         try {
+            //TODO: only works for users on server I guess, pretty bad way to find user, parsing id, back to IUser object, why?
             return DISCORD_CLIENT.getUserByID(Long.parseLong(mention.substring(2, mention.length() - 1)));
         } catch (NumberFormatException e){
             throw new InputException("Please mention the user.");
@@ -135,8 +147,8 @@ public static IUser getUserFromMention(String mention){
 
     private static void issueReport(MessageEvent event, Exception e){
         RequestBuffer.request(() -> event.getAuthor().getOrCreatePMChannel().sendMessage("Something went wrong while executing your command, I am notifying my maintainer now."));
-        //TODO: all this funny language is not very helpful
-        RequestBuffer.request(() -> MAINTAINER.getOrCreatePMChannel().sendMessage("You moron, I just caught a " + e.getClass().getSimpleName() + " due to " + e.getMessage()));
+        //TODO: all this funny language is not very helpful: You moron, I just caught ..
+        RequestBuffer.request(() -> MAINTAINER.getOrCreatePMChannel().sendMessage("Caught a " + e.getClass().getSimpleName() + " due to " + e.getMessage()));
         e.printStackTrace();
     }
 }

src/main/java/com/github/nija123098/tipbot/commands/BalanceCommand.java

@@ -3,6 +3,7 @@
 import com.github.nija123098.tipbot.AbstractCommand;
 import com.github.nija123098.tipbot.Command;
 import com.github.nija123098.tipbot.Database;
+import com.github.nija123098.tipbot.utility.Config;
 import com.github.nija123098.tipbot.utility.TransactionLog;
 import com.github.nija123098.tipbot.utility.Unit;
 import sx.blah.discord.handle.obj.IUser;
@@ -24,20 +25,26 @@ public String getHelp() {
     public Command getCommand() {
         return (invoker, arguments, channel) -> {
             update(invoker);
+            //TODO: again, should be BigDecimal
             double amount = Double.parseDouble(Database.getValue(BALANCES_TABLE, invoker, "0"));
+            //TODO: due to no caching of the usd api call this command is slow
             Unit displayUnit = Unit.getUnitForName(Database.getValue(Database.PREFERRED_CURRENCY, invoker, "USD"));
             return Unit.displayAmount(amount, 4) + " Dash which is worth " + displayUnit.display(amount / displayUnit.getDashAmount());
         };
     }
 
+    //TODO: Use explicit scoping instead of the default package private level
     static void update(IUser user) throws IOException, InterruptedException {
         String receivingAddress = Database.getValue(RECEIVING_ADDRESSES_TABLE, user, null);
         if (receivingAddress == null) return;
-        Process process = new ProcessBuilder("dash-cli", "getreceivedbyaddress", receivingAddress, "6").start();
+        //TODO: waiting for 6 confirmation is madness, thought this thing is broken at first, horrible user experience and for testing
+        Process process = new ProcessBuilder("dash-cli", "-rpcuser="+ Config.RPC_USER, "-rpcpassword="+ Config.RPC_PASS, "getreceivedbyaddress", receivingAddress, "1").start();//"6").start();
         String s = new BufferedReader(new InputStreamReader(process.getInputStream())).readLine();
-        if (s == null || s.startsWith("e")) return;
+        if (s == null || s.startsWith("e")) return;//TODO: bad way of error checking
         String previous = Database.getValue(BALANCES_TABLE, user, "0");
+        //TODO: all amounts should be BigDecimal, using double is not safe for crypto!
         Double addToBalance = Double.parseDouble(s) - Double.parseDouble(previous);
+        //TODO: we should display mDASH as 1 DASH is too much for most purposes, even 1 mDASH is almost $1 by now ..
         if (addToBalance < .000001D) return;
         TransactionLog.log("adding " + addToBalance + " to balance for user " + user.getStringID());
         Database.setValue(RECEIVED_TABLE, user, s);

src/main/java/com/github/nija123098/tipbot/commands/DepositCommand.java

@@ -3,6 +3,7 @@
 import com.github.nija123098.tipbot.AbstractCommand;
 import com.github.nija123098.tipbot.Command;
 import com.github.nija123098.tipbot.Database;
+import com.github.nija123098.tipbot.utility.Config;
 
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
@@ -20,7 +21,8 @@ public Command getCommand() {
         return (invoker, arguments, channel) -> {
             String address = Database.getValue(RECEIVING_ADDRESSES_TABLE, invoker, null);
             if (address == null){
-                Process process = new ProcessBuilder("dash-cli", "getnewaddress").start();
+                //TODO: this is very bad for performance, no error checking, tons of things can go wrong here with starting a process and hoping it al works
+                Process process = new ProcessBuilder("dash-cli", "-rpcuser="+Config.RPC_USER, "-rpcpassword="+ Config.RPC_PASS, "getnewaddress").start();
                 address = new BufferedReader(new InputStreamReader(process.getInputStream())).readLine();
                 Database.setValue(RECEIVING_ADDRESSES_TABLE, invoker, address);
             }

src/main/java/com/github/nija123098/tipbot/commands/LogoutCommand.java

@@ -11,6 +11,7 @@ public String getHelp() {
         return "Shuts down the bot";
     }
 
+    //TODO: not sure why this is needed, the maintainer has full control over the bot anyway
     @Override
     public Command getCommand() {
         return (invoker, arguments, channel) -> {

src/main/java/com/github/nija123098/tipbot/commands/PingCommand.java

@@ -5,6 +5,7 @@
 import com.github.nija123098.tipbot.Main;
 
 public class PingCommand extends AbstractCommand {
+    //TODO: this is an example command, not needed for a tip bot!
     @Override
     public String getHelp() {
         return "Pong.";

src/main/java/com/github/nija123098/tipbot/commands/RequestCommand.java

@@ -5,6 +5,7 @@
 import com.github.nija123098.tipbot.Main;
 
 public class RequestCommand extends AbstractCommand {
+    //TODO: not needed for the tip bot!
     @Override
     public String getHelp() {
         return "Sends a request to the maintainer.";

src/main/java/com/github/nija123098/tipbot/commands/SetCurrencyCommand.java

@@ -7,6 +7,7 @@
 import com.github.nija123098.tipbot.utility.Unit;
 
 public class SetCurrencyCommand extends AbstractCommand {
+    //TODO: I would keep it all in dash, display values in usd like everywhere else in crypto
     @Override
     public String getHelp() {
         return "Sets the currency to print value for.";

src/main/java/com/github/nija123098/tipbot/commands/TipAnnouncementsCommand.java

@@ -6,6 +6,7 @@
 import com.github.nija123098.tipbot.Main;
 
 public class TipAnnouncementsCommand extends AbstractCommand {
+    //TODO: also not needed
     @Override
     public String getHelp() {
         return null;

src/main/java/com/github/nija123098/tipbot/commands/TipCommand.java

@@ -20,6 +20,7 @@
 import static com.github.nija123098.tipbot.Database.BALANCES_TABLE;
 
 public class TipCommand extends AbstractCommand {
+    //TODO: just makes it confusing, all this bot should support is !tip !deposit !balance and !withdrawal
     @Override
     public List<String> getNames() {
         return Arrays.asList("tip", "give", "gift", "donate", "grant");
@@ -41,18 +42,24 @@ public String getFullHelp() {
                 "    " + Main.PREFIX + "tip " + Main.MAINTAINER.mention() + " .01 Dash";
     }
 
+    //TODO: Code issue: The method getCommand() has an NPath complexity of 2160
     @Override
     public Command getCommand() {
         return ((invoker, arguments, channel) -> {
             BalanceCommand.update(invoker);
             List<IUser> recipients = new ArrayList<>();
             IUser user;
             for (String argument : arguments) {
-                user = Main.getUserFromMention(argument);
+                //TODO: no idea how the user should figure this out, only works if <@username> is used, not simple mention and not @username (default in discord)
+                //TODO: also wasteful to check all arguments, this should be a regex
+                user = Main.getUserFromMention(argument);//TODO: only format that works is <@id>
+                //TODO: why break the loop? couldn't the user be mentioned later?
+                //TODO: Code issue: Avoid using a branching statement as the last in a loop.
                 if (user == null) break;
                 recipients.add(user);
             }
             for (IUser recipient : recipients){
+                //TODO: doesn't work, our user is the user id, not the name typed in chat
                 if (recipient.equals(invoker)) return "You can't tip yourself.";
                 if (recipient.isBot()) return "You can't tip a bot";
             }
@@ -84,6 +91,7 @@ public Command getCommand() {
             return Main.OK_HAND;
         });
     }
+    //TODO: Code issue: Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.
     private static final Map<Long, Double> TIP_AMOUNT = new HashMap<>();
     private static final Map<Long, IMessage> TALLY_MESSAGE = new HashMap<>();
     public static void handleReaction(ReactionAddEvent event) throws IOException, InterruptedException {
@@ -119,8 +127,10 @@ private static String completeTransaction(IUser invoker, IGuild guild, double am
         }
         return null;
     }
+    //TODO: would be better if all this is done in a public channel, not special some announcement channel
     private static void tipAnnounce(IUser invoker, IUser recipient, IGuild guild, double amount, Unit unit){
         String channel = Database.getValue(Database.ANNOUNCEMENT_CHANNEL, guild, "NULL");
+        //TODO: code issue: Position literals first in String comparisons
         if (channel.equals("NULL")) return;
         IChannel dest = guild.getChannelByID(Long.parseLong(channel));
         if (dest != null) dest.sendMessage(invoker.mention() + " tipped " + recipient.mention() + " " + unit.display(amount));

src/main/java/com/github/nija123098/tipbot/commands/WithdrawCommand.java

@@ -3,8 +3,10 @@
 import com.github.nija123098.tipbot.AbstractCommand;
 import com.github.nija123098.tipbot.Command;
 import com.github.nija123098.tipbot.Database;
+import com.github.nija123098.tipbot.utility.Config;
 import com.github.nija123098.tipbot.utility.TransactionLog;
 
+//TODO: unused import
 import javax.xml.crypto.Data;
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -43,8 +45,9 @@ public Command getCommand() {
             return ret;
         };
     }
+    //TODO: kinda important code, no error checking, reading dash-cli output and sending it back? not very good
     private static String sendMoney(String address, Double amount, String userID) throws IOException, InterruptedException {
-        Process process = new ProcessBuilder("dash-cli", "sendtoaddress", "\"" + address + "\"", String.valueOf(amount), "\"Standard Withdraw\"", "\"" + userID + "\"", "true").start();
+        Process process = new ProcessBuilder("dash-cli", "-rpcuser="+Config.RPC_USER, "-rpcpassword="+ Config.RPC_PASS, "sendtoaddress", "\"" + address + "\"", String.valueOf(amount), "\"Standard Withdraw\"", "\"" + userID + "\"", "true").start();
         BufferedReader inputReader = new BufferedReader(new InputStreamReader(process.getInputStream()));
         BufferedReader errorReader = new BufferedReader(new InputStreamReader(process.getErrorStream()));
         process.waitFor();

src/main/java/com/github/nija123098/tipbot/utility/Config.java

@@ -13,6 +13,8 @@ public class Config {
     public static String DB_HOST = "host";
     public static String DB_NAME = "name";
     public static String DB_PORT = "port";
+    public static String RPC_USER = "user";
+    public static String RPC_PASS = "pass";
     public static String TOKEN;
     public static Boolean setUp() throws IOException {
         Map<String, String> map = Files.readAllLines(Paths.get("config.cfg")).stream().collect(Collectors.toMap((string) -> string.substring(0, string.indexOf("=")), (string) -> string.substring(string.indexOf("=") + 1, string.length())));
@@ -22,6 +24,8 @@ public static Boolean setUp() throws IOException {
                 //TODO: should be trimmed, any space in the config and things don't work well anymore
                 field.set(null, value);
             } catch (IllegalAccessException e) {
+                //TODO: Code issue: Avoid throwing raw exception types.
+                //TODO: also not helpful to find what is actually wrong
                 throw new RuntimeException(e);
             }
         }));

src/main/java/com/github/nija123098/tipbot/utility/TransactionLog.java

@@ -13,6 +13,7 @@ public class TransactionLog {
         PATH = Paths.get("Transaction-Log-" + System.currentTimeMillis());
     }
     public static void log(String log) throws IOException {
+        //TODO: don't get why this is dumped into a file when we are using db elsewhere
         Files.write(PATH, Collections.singletonList(log), StandardOpenOption.APPEND, StandardOpenOption.CREATE);
     }
 }