Hot answers tagged

45

Firstly, Python has a style guide and (unless you're given a different guide, in which case please provide a link to it) you should follow it. Your code generally follows it, but note that the imports are in the wrong order, it should be: from collections import Counter import re import sys import numpy as np Note alphabetical order, and a split between ...


30

This kind of trace logging could conceivably be solved with an Aspect. Aspect oriented programming allows you to add boilerplate-style code to your program without actually writing the boilerplate inline. You could, for example, add an aspect than logged a method name and parameters every time a program entered a method, and then logged the method name and ...


26

Lots of good code comments already given. I'll focus on non-code aspects. Take this from a DevOps engineer that regularly troubleshoots complex, unfamiliar systems from logs under time pressure. You want all logs to always be consistently formatted and to have timestamp and location (source file and line) by default, it looks like you request the date with ...


24

Great idea. I was having the same problem and this helped me solve it. Your method for doing cleanup though is wrong (as you mentioned it might be). Basically, you need to close the write end of the pipes after passing them to the subprocess. That way when the child process exits and closes it's end of the pipes, the logging thread will get a SIGPIPE and ...


23

Reusable methods Notice that you have several methods that essentially do the same thing. Log and both writeCustomErrorLog methods follow the same approach: StreamWriter sw = null; try { sw = new StreamWriter("some file path", true); sw.WriteLine("some string"); sw.Flush(); sw.Close(); catch (Exception ex) { throw ex; } This can ...


20

Looking at your code, I believe that your problem is not only that the logging lines are ugly but they are a risk in costing you more than helping you: Like comments, log line maintenance is often forgotten, and might break your code upon changes, or - worse - confuse you when trying to troubleshoot by reading the log files (think of a situation where you ...


20

I see a number of things that may help you improve your code. Separate interface from implementation The interface goes into a header file and the implementation (that is, everything that actually emits bytes including all functions and data) should be in a separate .cpp file. In this case virtually everything in datetime.h and all static functions in ...


18

I see two more security issues with this code. Executing a string rather than an array of command and parameters When building a command line programmatically, it is dangerous to build it as a string, especially when any of the parameters comes from user input. When calling Kernel#system with a string, a shell interprets the command line; when calling it ...


17

Do not call main recursively. You are setting yourself up for stack overflow. Consider instead def main(): while True: try: your_logic_here except Exception as e: your_logging_here Testing for counter == 4 is better done in a loop: for _ in range(4): handle_acceleration handle_the_rest An ...


14

Making functions in header files static means that each translation unit gets its own separate definition. This usually isn't what we want. Standard practice is to declare the function in the header, and then define it in a .cpp file, e.g.: // header: #include <string> namespace lwlog { namespace datetime { std::string ...


13

You register keydown events for Control and Alt keys, but don't note when they are released. Therefore, you would not be able to distinguish between r and Controlr by reading the transcript. Those are some huge switch statements. I would consider making some lookup tables instead. There are fewer than 255 key codes defined, and their numeric values are ...


13

The Design A LogReader reads a log, filters the unwanted stuff and converts it into LogEntries. According to your design. That's a bit much. Split it up. Make one LogReader that just reads the file and presents you with endless strings. Oh wait, that's a BufferedReader! I'd see the LogReader basically as an iterator wrapper for BufferedReader. Make a ...


12

Efficiency the biggest improvement will be to not open the config file each time you log something, but only once when instantiating the logger. you could also open the log file only once, but it might not be a good idea to have it open for the whole lifetime of the program. create only one instance of the two SimpleDateFormat objects and store it in a ...


12

There's no way to tell this in a friendly manner: your indentation is abysmal. Let's fix that up first. Note that there's plenty left to improve, like giving your operators some space, but I'll leave that as an exercise for yourself. Keep in mind space is cheap and readability is very important in code. If you can add readability by using some extra ...


12

Have you already executed the code to see how it performs and if the battery will last? There is that famous Donald Knuth quote saying premature optimization is the root of all evil (or at least most of it) in programming. I never had to think about the energy consumption of a program, so I cannot tell you about the power efficieny. But as vnp already did, ...


11

You have some global variables in main.c: bool invisible = true; char fileName[MAX_PATH]; They should be placed in the closest local scope possible and passed to functions as needed. Your #defines in keylogger.c: #define VK_VOLUME_MUTE 0xAD #define VK_VOLUME_DOWN 0xAE #define VK_VOLUME_UP 0xAF can be made into a more concise enum: typedef enum { MUTE=...


11

I'll just go through this from top to bottom /** * Returns the next log entry. * * The NotReadableException has more information available. * You can recover the lines that could not be read by calling NotReadableException#getLines. * You can see which exceptions were thrown internally by calling NotReadableException#...


11

You're a Python native, aren't you? I can tell from the """ multiline strings and the four-space indents and really unidiomatic whitespacing and if/then/else/end instead of ternary and if not instead of unless and use of blocks instead of statements and wow this is really unidiomatic ;-; Each tip assumes that you've already applied the last. Also, I'm ...


10

I don't see why you say <- "decreases readability", but you could line them up if you think that helps (this style is typical of Haskell programmers) main = forever $ do diff <- user1 `subtractCounts` user2 timestamp <- formatTime defaultTimeLocale dateFormat <$> getZonedTime homeDir <- getHomeDirectory let logMessage = ...


10

+ in regex means Once or more. I think in your case you are mistaking it for a concatenation operator. I think you could factorize a lot with the help of better regex. For example: method_regx = re.compile("(method=)+[A-Z]+[\s]+") # ... k = method_regx.search(my_data) try: line_method = k.group(0).split("=")[1].strip() except: Could be rewritten ...


10

Security I'm not all that comfortable with echoing unsanitized user input: echo '<form action=' . $_SERVER['REQUEST_URI'] . ' method = "post">';. I did not find any way to exploit it though. I asked a question on security.SE, maybe they can find a way. I would probably use SCRIPT_NAME instead. you should regenerate the session id via ...


10

1. Design The decorator mechanism seems hard to use, unreliable, and counter-productive. It's hard to use because you have to remember to add it to every method that might raise an exception. It's unreliable because it would be easy to forget one or two. And it's counter-productive because in Python, exceptions are not only used for runtime errors, but for ...


10

Duplicated code: if not fname == "": folderPath.delete(0, END) ## This is to erase anything in the "browse" bar folderPath.insert(0, fname) ## and use what has been selected from the selection window else: folderPath.insert(0,...


10

Raising useful errors If you have something like this: raise You're doing something wrong. Just calling raise with no additional useful info is not helpful. Preferably, you should do one of the following: Raise a built-in error: raise BuiltInError Raise a built-in error, with an error message: raise BuiltInError("Blah foobar") Define your own custom ...


10

I'll start off by saying this is actually really good. There are a few minor PEP8 errors: In after_login you call Request. the argument url shouldn't have a space on the right side of the equals. Another thing in Request. The argument callback needs at least one more indent. However you should either aligned with opening delimiter or use hanging indents. ...


10

First some comments. Usage of Deprecated Features Currently your trigger does a SELECT that returns a resultset from a trigger. That was at one time supported but has been since deprecated since at least 2012. I would remove them. Audit requirement I very much agree with @Austin Hasting that it's generally better to not transform the data into a ...


10

You have some good answers already. My criticism comes from one word. "printf". The problem for fault-finding is not just knowing what each part has reported, but also knowing in what order they happened. Any practical logging library is going to hit threads sooner or later, and at that point things go wrong. For starters, we need to think about thread-...


10

A few more suggestions in addition to the great existing answers: Support logging stack traces - it's easy! Standard C++ offers no facilities for obtaining stack traces, so traditionally - logging libraries and manual logging have foregone those. But these traces are extremely useful in inspecting logs and debugging programs (despite their verbosity); and ...


9

As Bill Michell has pointed out aspect oriented programming is a good way of adding boilerplate logging code, however in this case I don't think its really necessary - I'd approach this problem with some good old fashioned refactoring. In your example the Foo method does all the logging, but if it is importantly to log that you are about to do something (or ...


9

I am not convinced that your general structure is useful as it should be. I have been trying to untangle how your code us used, and what sequence of events happen in order for the code to run the way it should.... I have run in to a number of issues in that process, and they all add up to a suggestion that you are using the wrong approach. Even with your ...


Only top voted, non community-wiki answers of a minimum length are eligible