# Tag Info

75

I assume you're just trying to avoid the error message in case it doesn't exist. What if you just ignore it: Remove-Item $folder -Recurse -ErrorAction Ignore If that's not what you want, I would recommend writing your own function: function Remove-ItemSafely { [CmdletBinding(SupportsShouldProcess=$true)] param( [Parameter( Mandatory=$true, ... 24 A few things I would do differently: static char * ReadAllBytes(const char * filename, int * read) { ifstream ifs(filename, ios::binary|ios::ate); ifstream::pos_type pos = ifs.tellg(); // What happens if the OS supports really big files. // It may be larger than 32 bits? // This will silently truncate the value/ int length = pos; ... 18 I'm not sure why this is async. Unless you have a good reason for write to the file asynchronously, this is wasted effort. If you lock properly, this will work fine. Your locker variable should be marked static. You can then get away with something like this: using System; using System.IO; using System.Text; using System.Threading; using System.... 18 Python is 'batteries included', this means offloading code to the core library is a good thing, and often possible. In this case, if you're running Python 3.4+ you could make use of pathlib. All you need is to know the pathlib.PurePath constructor and PurePath.parts. And so you can change your code to: import pathlib def cut_path(path): parts = list(... 17 General Now that you have such neat postings, the answers are going to need to be neater too. GUI Bugs When I run the GUI, it does not let me select directories from the File Browser. It also starts in the 'Documents' directory, and it would be better to do one of two things: start in the current directory start in the last directory used (use java.util.... 16 You can simplify paths using forward slashes: GOOGLE_DRIVE_DIRECTORY = 'C:/Users/Jeff/Google Drive/Manifest_Destiny' MAIN_BACKUP_DIRECTORY = 'C:/Users/Jeff/Desktop/Manifest_Destiny_Backups/md_backup_{0}' EXTERNAL_DRIVE_DIRECTORY = 'F:/My Files/Manifest_Destiny_Backups/md_backup_{0}' Instead of converting a date to a string, which may be locale dependent ... 14 First, you don't need to call Get-Date for every file. Just call it once at the beginning:$t = (Get-Date).AddMinutes(-15) Get-ChildItem -Path $path -Recurse | Select Name, PSIsContainer, Directory, LastWriteTime, Length | where {($_.LastWriteTime -gt $t)} That's saves about 10% (as measured by Measure-Command). Secondly, you don't need to call ... 12 Why don't you use Directory.EnumerateFiles and Directory.EnumerateDirectories methods? There is an overload which takes SearchOption enum and lets you query subdirectiries too. static IEnumerable<string> EnumerateFiles(string root) { return Directory.EnumerateFiles(root, "*", SearchOption.AllDirectories); } static IEnumerable<string> ... 12 1. Comments on your code I like the way you've used docopt. But why do you require the user to run rename-flac.py -o in order to read the documentation for the schemes? Surely it would be clearer and simplier if the documentation for the schemes were included in the usage documentation? Your program would be easier to test if you put the top-level code into ... 12 Well, this is actually pretty decent code. There still is a lot of stuff that could be improved, I tried to focus on some more relevant ones. Subroutine Prototypes In a declaration like sub foo ($@&*) { ... }, we call the weird thing in parens a prototype. The prototype primarily changes how a call to that sub is parsed, and can set properties like ...

12

Your problem is not with 5 (or more) loops. Your problem is that you mixing code of different nature: walking over filesystem and name matching, and processing of files into single chunk of code. Separate it into different functions calling each other: def process_worksheet(wb, worksheet_name): sheet = wb.sheet_by_name(worksheet_name) # ... def ...

11

Style Minor nitpicking: your style is not 100% consistent. int main(int argc, char** args){ // should be int main(int argc, char **args){ while(1) { // should be while(1){ Pattern I think this block is very, very unusual. switch(parents){ case false: remove_dir(path, verbose); break; case true: do_parents(path, verbose); break; } A switch ...

11

My overall take was "Nice!". :) Given your exchange with Patrick J S above (not to mention my lack of familiarity with the wrinkles discussed in the relevant RFCs) I'm not going to comment at all on the technical-compliance-with-standards aspects of your decoding logic. Simplify Inline::Perl5 code use Inline::Perl5; my $p5 = Inline::Perl5.new;$p5.use('...

11

In my testing, constructing one Perl script and running it repeatedly is much faster (0.5s versus 3.6s) then running a new Perl instance for each replacement: while read f; do echo "s/$f/$f/gi;" done < files.txt > s.pl while read fn; do perl -pi s.pl "$fn" echo "$fn | $count/$lines finished..." count=$((count+1)) done < filenames.... 11 I would pinpoint the major problems in the code which I would address first: do not manually handle provided command-line arguments - there is a great built-in module for it - argparse. It would seriously simplify the code and improve the overall readability and scalability (adding more arguments would be easy). And, it also can generate help automatically ... 11 file1_path == file2_path surely tells that paths refer to the same file. However, even if file1_path != file2_path they still may refer to the same file. file1_exists = std::filesystem::exists(file1_path); introduces a TOC-TOU race condition. The file may exist at the time of test, yet disappear by the time of use. See the next bullet. std::filesystem::... 10 Overall your code is very well formatted and readable but I have some complaints at the moment: Don't declare all methods with throws Exception. Only declare the Exceptions you really need to declare, such as IOException. Declaring throws Exception will force the caller to also catch all RuntimeExceptions, which I say is a big no-no as that could ... 10 You do not use file_list Emptiness of container (as in PEP8) is usually checked by if not container. For path operations os.path is preferred. Let's change 'final' to os.path.dirname and building path to os.path.join. Avoid os-specific solutions. Let's reduce indentation by using continue statement. IMHO using comprehensions is not preferable here. Pythonic ... 10 I don't have time for a full review at the moment but something filled me with rage jumped out at me... Naming The code should be readable - you have 2 ways of making that the case, one is by naming things correctly (the second is formatting). So when I see things like this: int a = 0, ac, b=-1, li = 0; // a = current folder marker, ac = current file ... 10 This program is really a pleasure to read. It accomplishes the desired outcome neatly and succinctly. The only thing I could think of that might be an improvement is threading. Other than that this program looks fantastic. I can see where making this program threaded can be a great benefit when dealing with smaller files. I'm not sure, but I fear with ... 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 First of all, you should definitely read the Python style guide (PEP0008). It contains tons of good formatting and style info to make code more readable and neat for you and other users. I'll reference some of the points relevant to you but the whole page is good to commit to memory. You should rearrange the imports so they're grouped for readability. Plain ... 10 A common problem with file archive extractors is vulnerability to directory traversal attacks. A maliciously crafted archive could contain an entry with a path like ../../../../../../../etc/resolv.conf, which could overwrite a system file if the extraction is done by a privileged user. You should take measures to ensure that any file you create lies within ... 10 I will not mention again that I find the outdenting of Dim statements hurt readability. Instead I'll focus on this part: On Error Resume Next Set wbTest = Workbooks(strTargetName) IsWorkbookOpen = (wbTest.Name = strTargetName) On Error GoTo 0 What error are you expecting to trap here? Workbooks(strTargetName) This will raise runtime error 9 - ... 10 This is not efficient pathString += String is immutable so it builds a new string every time Use StringBuilder 10 Here are some suggestions for how you might improve your code. Use the appropriate #includes This program fragment requires headers, which should be included: #include <stdio.h> #include <stdlib.h> #include <string.h> Use const where practical Because the validate_path function does not alter the passed string, the parameter should be ... 10 You can split each path at / using split and then use reduce() to build that object. var paths = [ 'RootFolder/FolderA/FolderB/FileA', 'RootFolder/FolderA/FolderB/FileB', 'RootFolder/FolderA/FolderB/FolderC/FileA', 'RootFolder/FolderA/FolderB/FolderC/FileB' ] var obj = {} paths.forEach(function(path) { path.split('/').reduce(function(... 10 Do not silently ignore invalid arguments Your script allows to specify the source directory (and other locations). Here is the relevant part in function parseArgs(): -s|--src) if [ -d "$2" ]; then srcDir="\$2" fi shift 2 ;; If --src <sourceDir> is specified ...

10

Instead of checking with an if statement if errStat is null and then returning false: if errStat != nil { return false } return true you can return a Boolean expression: return errStat == nil

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