# Tag Info

17

Well, you could find a "Pythonic" way to skip around a file, but that still wouldn't make it good code, in my opinion. There are some fundamental issues with your approach that make it rather hackish: f.seek(0) means that the file isn't being processed sequentially, and creates extra work. You're hard-coding ano = 1977 when in fact ANO 1977 would be right ...

10

Let's start with your priorities: optimization. You should be primarily optimizing for the most expensive and limited resource, which is almost always developer brain cycles. You optimize for that resource by writing clean, readable, well-organized code with few surprises. Your code can be improved on all of these fronts. As far as CPU or hard-drive ...

8

if (array.Length < count) { string[] newArray = new string[count]; for (int s = 0; s<array.Length;s++) { newArray[s] = array[s]; } for( int s = array.Length; s<count;s++) { newArray[s] = ""; } array = newArray; } You can use Array.Resize to simplify this. if (array.Length < count) { var ...

8

I don't think that using : is a bad practice but you have to escape it somehow if it occurs inside your data. Anyway, I'd consider using XML or JSON here. Some notes about the code: You should close the stream (in a finally block). See Guideline 1-2: Release resources in all cases in Secure Coding Guidelines for the Java Programming Language The .ser file ...

7

An alternative solution could be a binary reader (see also: this code project article) Implementation: (I just replaced the 2 try catch blocks with one using): public static void ReadBinaryReader(string filePath) { using (var stream1 = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.None)) { stream1.Position = ...

7

Have you ever heard about tussenvoegsels? They're parts of people's names. Well, in the Netherlands anyway. When used for authors, it's usually done as "van Surname, FirstName". Your regex doesn't support this, instead only accepting the last word of the surname. You should allow surnames to consist of multiple words. Dim IsValid As Boolean If Len(...

7

Well, why not use a loop? for i in range(13): next(f) You should also use constants instead of magic numbers, like this: LINES_BETWEEN_SIGN_AND_DATA = 13 for i in range(LINES_BETWEEN_SIGN_AND_DATA): next(f) More generally, make sure you comment the intent of the code since readers of your code might see the code but not the example files.

6

Is this method of implementing saving and loading reliable and efficient? It should be reliable. And if saving all of this information is necessary, then this is almost certainly the most efficient way to do it all. The way to improve efficiency further is to ask yourself what datapoints are absolutely necessary to write and read later, and what ...

6

Why are you using regex for this? It makes no sense to match complicated naming conventions with something as low-level as Regular Expressions. You either have to go higher in your abstraction by using a proper Grammar (which is basically what your separated groups do, but better) or go lower to forego the semantics you impose here. Consider the following ...

5

Two quick remarks: There's a typo, it should be Separator, not Seperator. Microsoft's standard is to use PascalCase for constants.

5

var match = Regex.Matches(fileContetnt, PATTERN, RegexOptions.IgnorePatternWhitespace | RegexOptions.Multiline); Because Regex.Matches() returns a MatchCollection you should use the plural form for naming the variable. var matches = ... Encoding encoding = Encoding.UTF8; //Encoding.Default; //if (Utf8Checker.IsUtf8(filestream)) // encoding = ...

5

#!/usr/bin/python Consider using #! /usr/bin/env python instead (or python3). Then you can manage import dependencies with venv or, better, conda env update. Conda will adjust PATH to point at the appropriate python interpreter which has the necessary pypi libraries that inevitably get added to a project. # By Neale Irons version 25/02/2018 (CC BY-SA 4.0) ...

4

Scanner is a bit easier to use then BufferedReader... import java.io.File; import java.io.FileNotFoundException; import java.util.ArrayList; import java.util.List; import java.util.Scanner; public class Main { public static void main(String[] args) { try { File f = new File("people.ser"); Scanner sc = new ...

4

This seems pretty nice. I have just some nitpicks about this part: postfixs = [".isf", ".ISF"] if os.path.splitext(binaryfile)[-1] not in postfixs: raise ValueError("File type unkown.") Nitpicks: I generally don't like repeating the same thing in lowercase and uppercase versions. I prefer to use lowercase, and convert untrusted input to lowercase. ...

4

I have a few minor comments, like "move the import statements to the top", but my biggest concern is the confusing structure. Why do you need a class? A warning sign that something isn't a class, or shouldn't be a class, is a class with two methods, one of which is __init__. Your init is even empty. Okay, your class has three methods, but I'd still count ...

4

Use expect instead of unwrap. This gives you and your users a better chance at figuring out what went wrong. It's also one step closer to good error reporting and handling. Extract an inner_main that returns a Result and thus allows you to use try!. main can call that method, pattern match on the Result, print the failure message, have an exit code, etc. To ...

4

Your problem isn't the struct reading but the reading of small junks from disc. Consider to load the whole file into a memorystream first and then use either your aproach or use the BinaryReader like @JanDotNet suggested. As a side note, you really should name your things better. E.g stream1 implies that there is at least another stream around. Swallowing ...

4

Overall As I mention in the comments. There is nothing that makes this code inherently C++ like. It basically looks like a C program that has been translated in C++; a few utility classes like std::fstream,std::string` and a few other are used incorrectly so you may as well have used the C counterparts and the program looks the same. You do not take ...

3

pe32.h Since you call Windows APIs, it is reasonable to expect that the program targets Windows platform. You really should take advantage of ImageHlp APIs and structures, and in any case not rely on the third party 5-year-old documents. Copying structures seems unreasonable. Data are already in memory, so setting up a pointer should suffice. For example: ...

3

To improve the performance, you should list the files in an Array instead of a Collection. I would also drop the unnecessary * for the file expression: Sub GetFileList(folder As String, outList() As String) Dim fname$, count& ReDim outList(0 To 10000) ' set initial array size fname = FileSystem.Dir(folder, vbNormal) Do While Len(fname) ... 3 _read_chunk() does not need to return the filepointer position as you never use it. After leaving the main loop you (have to) read the filepointer explicitely. Thus, current is oblivious. Speaking of naming, 'current' can be a lot of things...'currentposition' or the like would be self-documenting. Likewise, a 'bfile' and a 'binaryfile' might be an '... 3 The comment # Format info: http://lclevy.free.fr/cr2/ # The Cr2 class loads a CR2 file from disk. It is currently read-only. would be better in Cr2's docstring. "It is currently read-only" doesn't mean much since everything is public. To mark things "private" the convention is to prefix them with underscores (eg. self._header). I wouldn't do that, though; ... 3 On top of what has already been mentioned: Your implementation will break if any of the column names or values contains a ';'. Proper CSV parsing is a tad harder than most people naively assume. Now this may or may not matter because you might say "The data will never ever contain ;" however given the number of times this phrase was uttered and then proven ... 3 There are several things I don't know about your code and format of the data - in particular what's exactly stored in the Dictionary returned by GetRootNames, GetRootFolderNames, etc. In my example below, each of those dictionary keys stores the full path to a folder (so you'll have to make adjustments for your own situation). But I'm hoping the example ... 3 Heapsort drawbacks My review will be directed more to the algorithm chosen instead of the actual code. You've chosen to use heapsort, which allows you to easily compute a progress percentage. You also implemented a cache which lets you hold the top elements of the heap in memory. Accesses using the memory cache are much faster than accesses that need to ... 3 Complexity One method for determining how complex your code is, is called cyclomatic complexity. You basically count the number of branches in the code to determine its complexity. I count about 10 in the above code snippet. A cyclomatic complexity of 10 is often considered the maximum you should have in good readable code. (This is of course subjective.) ... 3 It looks like the choice of area to scan and iteration through the disk is outside the scope of this program, so we'll assume that this is a decent way of finding and recovering a jpeg file. Implementation: It looks like your program is set up to scan/find multiple images within the given region. As it is currently written, it will output 512 byte blocks ... 3 Instead of cat "$x" | command or echo "$x" | command, use command <$x (vs cat) or command <<<\$x (vs echo): it saves a fork and removes the need to quote. Instead of if [ x -lt y ] use if [[ x -lt y ]]: it saves a fork ([[ is a bash builtin; help test for details) and adds some functionality. Functions return their last exit value already so ...

3

Some suggestions: Every single line should at least be comprehensible on its own. Single letter variables destroy that comprehension, meaning that anyone reading the code needs to read the entire code and mull it over for a while to even understand what it's meant to do. Something like current_byte = file_handle.read(1) is a lot more readable than b = f....

3

You do a lot of manipulation of bytes to construct larger structures, which you then operate on. I'd suggest looking at the standard module struct which can do a lot of this for you in a way that is much easier to grasp. Also, if you're going to read some bytes and then read them a second time, why not just read them into memory and operate on them twice? ...

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