7

You could easily wind up with collisions on the folder name if the method is called multiple times in very quick succession because of the new Random() being declared for each usage. It should be instantiated once, and since you mentioned it could be called on multiple threads, it should be mutexed onto one at a time. Finally, it's not a good idea to have ...


5

Style feedback on how clear my comments were and how easy the code was to follow I'm not a big fan of line comments. I think they make the code a bit harder to follow. That's why I prefer tail comments along with a nice tabular layout of the labels, instructions, operands, and tail comments. And of course don't write redundant comments like "Update the ...


5

For what you are trying to achieve, Java has a very nice solution since Java 7: java.nio.Files. The Files class comes with a lot of useful methods that do what you did more expressively and easier to understand. Instead of listing all files in a directory and looping over them, you can actually walk the entire file tree, including sub-directories, if you ...


5

Don't use callbacks, it's hard to read and maintain, the fs.readdir function will return a promise if you didn't provide a callback function. Use arrow functions when you could. Respond with an error instead of throwing (unless you have a catch wrapper middleware). Use property shorthands. You forgot req and res. Try this: const express = require('express')...


5

Review Modelling the domain is crucial when designing a multi-layered application. Your concerns about polutting the domain with file or web specific code is pivotal to make a clean, extensible, reusable and testable API. I am finding it very difficult to think of a domain object which does not include FileInfo or FileStream. I would suggest the ...


5

at first I thought to write this as a comment, perhaps the comment would be a little to long… There are many things to consider here, not really inline with your question but perhaps you should consider them. Transactional scope. You update the database, remove records… you may end-up orphaning the disk images where you have images that are not ...


5

CodeReview Why use the Scripting.FileSystemObject? MyFolder is just returning the folder name MyFolder = Worksheets("Config").Range("D4").Value & ActiveSheet.Range("C21").Value I would also write a function to return the folder path and a second function to create the icacls commands. Fun Part: My Own Solution The class below will create a self-...


5

As you tagged this Python 3.x, I'd suggest using pathlib.Path.rglob() and collections.defaultdict. Also, check out f-strings. import collections import pathlib mp4s = collections.defaultdict(list) root = pathlib.Path(r'/mnt/mp4library/mp4library') file_count = 0 for filepath in root.rglob('*.mp4'): #filepath is relative to root. Uncomment the next ...


5

Why do the getters for Entry return const references? getEntryType, in particular, does not benefit at all from returning a reference since it just returns an enum value (which is the same size or smaller than what would be returned for the reference). getParentDirectory should be a const function. If the first three members of Entry are const members, ...


5

If there's no reason to make it a class, don't make it a class. (A good clue is that your class doesn't have any instance variables, and that both of its methods could be static without changing the way they work.) Both of your functions return the same information (formatted differently), but one calls it file_size and the other calls it directory_size ...


5

At first glance, here are the few things I saw that need to be fixed: Your functions seem pretty long, and this possibly leads to violation of single responsibility principle. Make sure each of them does only a single job. There are many magic numbers (such as using 0:3, 0:4 while slicing arrays). Same applies to your strings too. As someone who reads your ...


4

A few things I would change are: Some private methods tend to check for null on their arguments, even if you control, what they are being called with. Unless I plan to call with NULL, I'd remove those checks. The exception methods might either provide no message, as they do not help the caller more than the standard message of the exception or provide ...


4

some minore issues only... naming / return type public static Path getLatestFile() assumes that you return only ONE file, but it's easily possible that you have two files of the same date... so you should maybe return a Set<Path> from your method getLatestFiles(). This method would return all files with the latest time stamp. use a logging framework ...


4

OsStr conversion According to the path source, s.as_os_str().to_str() is the same as s.to_str(): pub fn as_os_str(&self) -> &OsStr { &self.inner } pub fn to_str(&self) -> Option<&str> { self.inner.to_str() } Moreover, using unwrap() here is not safe at all as invalid Unicode is pretty common (e.g. when mounting ...


4

Architecture DDD-wise : The most important thing about DDD is that the domain drives the design (so.. DDD). That means that when we look at your domain objects we should be able to understand the whole logic of your application. Let's summarize what your application is doing : A user posts an product for sale, where the product can have a Youtube link, a ...


4

Thank you for an interesting question 😊 Here is the first part of the answer, I will publish more over the weekend at https://github.com/dmitrynogin/shopless. Let’s talk about storage access first. You need to deal with different types of storages, so let’s abstract: public abstract class Folder : Enumerable<string> { public static Folder Open(...


4

Welcome to Code Review, nice first question. Don't Panic, and always take a towel with you. General Observations This may be a copy and paste error, but the level of indentation is inconsistent. This makes writing, reading and debugging the program very difficult. There are too many comments. Code should be self documenting for the most part, only things ...


4

Just reviewing the FileGroup class. 1. Review The name FileGroup does not convey the intended purpose of the class (namely, that all the files in the group share an inode). A name like InodeFiles would be better. The intention of the class is that all the files share an inode. But the class itself does not check this: it relies on the caller to do so. It ...


4

The loadData function looks just fine to me. I don't see a way it could be any more elegant given what it's doing. I do actually like the way you have separated reading the file and parsing it. That's the way to go. Your concern about "avoiding the IO monad" shouldn't be a concern. On the contrary, that's actually a "best practice", known as "pushing I/O ...


4

Just a few remarks from me: Rather than hardcoding the path in your code it would be better to use a command line argument, then it can be used like an OS command such as dir or du. That is more user-friendly since your aim is automation. There is no exception handling in your code, so it will crash on permission error, which could easily happen, then you ...


4

Style conventions Following the PEP style guide, class names should be named with CamelCase and documentation strings should be formatted like the following. class DirectoryStatistics: """Gets the total size of a given dir and counts how many folders and files are in the given path directory. Also offers a printing utility to output the ...


4

I have a writeOnFile() method with too many fwrite(). ... A way to reduce fwrite() calls is to simply write all text to a large char buffer[] and then write that once per for (int i = 0; i < global_index; i++) loop. Use the return vale of snprintf() to speed calculation of next offset. Use *printf() features for padding. for (int i = 0; i < ...


4

Python 3 classes You should omit (object) as the base class for classes in Python 3. Re-entrance changes is a global that's mutated by FolderSyncer, so immediately this is neither re-entrant nor thread-safe. Maybe move the changes list to a member of FolderSyncer. Anonymous tuples Your changes has a few other issues: The inner lists should be tuples, ...


3

Hello and welcome to Code Review! I try to give a possible explanation of what the reviewer meant when he was talking about inverse class structure; in the main method this code is present: MyAlgorithm myAlgorithm = MyBuilder.builder() .withCipherSet(CIPHER_CHAR_SET) .build(); But from your code MyAlgorithm is a subclass of ...


3

This is a review of your 2nd review request, described as a rewrite for python3. Summary: code looks great! Much better than the older version. Many changes I suggested for the old already appear in the new. Thank you for turning mangling double __ into private single _ prefix. 0,aaa,b,c,dd (Bit Mapping) The explicit "bit mapping" remark is very ...


3

DFS (Directory and File Serialization) file format Not sure what that is. Nor is google. Recommend you offer a more specific citation, hopefully one that includes an URL. import os as _os import sys as _sys Hmmm, that's odd. Do you really need to rename them? FORMAT_DOCUMENTATION = '''\ The backwhack is very odd -- don't do it. That's what triple quotes ...


3

File class: rw_s(T &value, size_t arr_size = 1); ... template<typename T> rw_s<T> rw_soft(T &value, size_t arr_size = 1); ... File &read(T *val, size_t data_size = 1); The default size argument is likely to cause problems. I'd suggest separating the interface for reading / writing arrays from single values. Reading / writing a ...


3

Don't reimplement functionality that is already provided by the standard library Why do you need a FilesManager? It is just a glorified std::map. The only extra features it has is a limit for the maximum number of files, and that you can specify a base directory. In my opinion, that's not enough to warrant this class. Why would you ever want to set a limit ...


3

In all, it's a good effort. Here are some things that may help you improve your program. Choose appropriate variable types The keep_running flag is a std::sig_atomic_t but since you're using C++17 anyway (for std::filesystem), I'd recommend making that an atomic_bool which more clearly reflects its usage and intent. Use const where practical The ...


3

Like you noticed, no need to use multiple loops when you can do everything in one function and loop only once. Aside from that, there are many small things that can be improved with this script: This script doesn't work if the script is not in the same directory as the source directory. Correct if me I am wrong, but this is what I encountered when I tested ...


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