Hot answers tagged

7

Use modern versions of things The most obvious thing here is use Python 3. This will help massively with your Unicode problems, because Python 3 maintains a stricter separation between things that Python 2 conflated. In some cases, your errors will just be artifacts of Python 2's way of doing things, and will just go away. In others, you'll get errors that ...


7

Here are my main recommendations: use recursion use names instead of list indices, for example node$data$reply$data$children reads much better than node[[2]][[5]][[2]][[2]] and it is also more robust to data changes. use well-named variables so you code reads easily Now for the code: url <- "http://www.reddit.com/r/funny/comments/2eerfs/...


6

1. General issues r.login ('','', disable_warning=True) #notice True is boolean and not string if you did disable_warning='False' it would still disable warning. Every non-empty string in python when cast to boolean will return True. Too many global objects. Reddit API client can stay global r, having current_time as global is horrific. Stuff like ...


5

A few suggestions for general code quality: If your class only does one thing and it doesn't store any values, it should be a function. Redpy only downloads images from reddit and stores values to achieve exactly this, which you could do in a function. Using a class can have unforeseen consequences. Choose descriptive names for variables and functions. ...


5

Pass values as parameters for modularity You can make your code a lot more modular if you pass values to functions as parameters instead of hardcoding values in the function itself. Let's change the bot_login() function to this. def bot_login(client): print("Login...") reddit = praw.Reddit(client_id=client["id"], client_secret=...


5

Note that the condition if word in words in replacer performs a linear search on words if it's a list, as in the current code. Since this condition is executed for every word in the input, it would be good to optimize it a bit. You could for example pass a set of words instead of a list. The names first_iter and second_iter are not very meaningful. It would ...


5

Yes, there is a more concise way to do it - using CSS selectors and a list comprehension: top_subreddits = [ td.get_text() for td in soup.select("tbody tr td") if '/r/' in td.text ] tbody tr td would locate all td elements under tr elements which are under tbody. I don't really like getting texts of td elements twice here, there is possibly a ...


5

Invoke((MethodInvoker)delegate { bool exists = false; foreach (ListViewItem item in listView1.Items) { if (item == lvi) exists = true; } if (!exists) listView1.Items.Add(lvi); }); This single snippet of code has many things that can be improved in both readability, performance, and maintainability. First, ...


5

public static bool DownloadImage(string url, bool isDirect) // returns if repost C# supports something called XML Documentation Comments which allow you to document your code directly while writing it in a processor (as in IDE) friendly way. public static bool DownloadImage(string url, bool isDirect) // returns if repost Liar! This does not download the ...


5

The issue probably isn't with your code. An operation like this is almost certainly IO bound, not CPU bound. This means that the bulk of the time is being used waiting for the network to respond, not waiting for your CPU to process something locally. You can speed a program like this up by multithreading it. This strategy would allow you to have multiple ...


4

Some comments based on a brief skim: If you want to write Python, you should read PEP 8, the Python style guide. Among other things: Python variables are lowercase_with_underscore, not camelCase. Lines should be wrapped to 79 characters wide. Module imports should be in alphabetical order, and split into groups depending on whether they’re standard library,...


4

Never do except Exception:, especially not to pass. You probably want to be catching KeyErrors specifically, or using dict.get in the first case, and in the other cases adjust accordingly. There's no need to do json.loads(file_like_object.read()), just use json.load(file_like_object). You can eliminate a race condition in makeSavedir by not calling os.path....


3

First of all, naming: do not abbreviate names unless they are used in a one-liner like [x.something for x in somelist]. Do not try to spare characters - use IDE to suggest continuations if you want to type fast. Really pays off. Second, try not to mix abstractions of different types in the same function, eg, write_comments_to_file deals now with preparing ...


3

Regarding your question about the replacer function, you can shorten it a bit by using list comprehensions: def replacer(title, words, randoms): """ :param title: string you want to split. :param words: list of words you are looking to match. :param randoms: list of random words to replace with words. :return: new title """ ...


3

var domains = document.getElementsByClassName('domain'); for (var i = 0; i < domains.length; i++) { You can covert document.get* results into real arrays by using array.slice. That way, you can take advantage of the handy array methods like forEach. if (data.crappyAction === 1) { 1 and crappyAction doesn't really tell me anything about what the code ...


3

Some points about your code extract the code to create an account to a separate class. In this way you can reuse it without problems. This can even be a static method. variables should be named using camelCase casing. See the naming guidelines. as you don't use neither the TemporaryCookie nor the CreateAccountCookie you should remove these varaibles. ...


3

My main comment is that you should separate the different concerns of you code into descriptive functions. This will make it a lot more readable (and re-usable). One comment before I get started: Your login function, which looks like it would log you in with the Reddit API seems to be unused at the moment. I guess this is a copy&paste error from ...


2

I would move your jam_text text into a separate file or a multiline constant string to keep the test itself cleaner. Also, this block: jazz_piece = None # type: Piece for selection in selections: if selection.title == "Jazz Title": jazz_piece = selection break can be simplified via using next(): jazz_piece = next((selection for ...


2

As you're on Python 3.6+, you should definitely use function annotations and type hints to make your code more understandable to others. any(ex in url.lower() for ex in ['.png', '.gif', '.jpg', '.jpeg']) can be replaced with url.lower().endswith(('.png', '.gif', '.jpg', '.jpeg')). Another option is to use the pathlib module and do: Path(url).suffix in {'.png'...


2

There are few things you can do. Start by encapsulating these lines if (imageURL.Contains("gfycat.com")) { imageURL = imageURL.Replace("gfycat.com", "zippy.gfycat.com") + ".mp4"; } if (imageURL.Contains(".gifv")) { imageURL = imageURL.Replace(".gifv", ".mp4"); } in their own method. Let's call it FixImageUrl (this is C# 7) public static string ...


2

The main downfalls of speed in your code are going to come from constantly writing and reading a file. I think the best option to reduce this and ultimately speed up your program would be to Read the file (as you do). Split the file (as you do). Store the array globally. Do not write back to the file. This is going to speed up your code because you aren'...


2

I think this code is very straightforward and easy to read. Good job! Some nitpicks are, You have a few PEP8 violations none all that concerning. Should add a second white line between imports and your functions Line too long, at your print statement Continuation line under-indented for visual indent printf style formatting is recommended against Code ...


2

Nesting promises defeats the purpose of promises as a whole. Promises can be chained together. This is done by returning a promise to then. function postTwit() { return saveImage(content.url, PATH) .then(() => { // Return a promise to a then. The promise returned by then // will not resolve until this promise resolves. return T....


1

Well, the methods are not that similar... basically, the real duplication is just reading a URL and parsing it into a json array, where the only difference is the message you print to stdout. As System.out.println() normally is just a temporaty solution (like: c'mon, who will read your stdout in reality?) I wager you can simply leave this out or replace it ...


1

This is much clearer now (hopefully, you also agree). Some more ideas: It is sometimes considered cleaner to always define functions before their use. Some old comments still apply (magic number -6 and some namings), but there may be reasons for not changing that. Code now almost reached the top of possibilities for non-object-oriented methodology, so I ...


1

Overall, it gets the job done, but your code (especially the area I'd refer to as "pyramid code"), could use a little more organization and clarity. Your if statement could be a little more concise, and, at the same time, more canonical about what it's actually there for: if (!('REDDIT_USERAGENT' in process.env)) { ... } You can write concise code ...


1

It doesn't make a whole lot of sense to have a function with no arguments that modifies a global object. Your init_reddit function is better than your get_reddit function because of this. IMHO you should rethink why you have a function that has more comments than code in it. There may be a more idiomatic way to express that. (see find_piece_matching_title, ...


1

Your code is very clean, easy to understand and well written. My only comment for improving it would be to take advantage of a framework that does some of this lifting for you, such as: KnockoutJS BackboneJS AngularJS As well, coming from an OO background, you might be interested at looking at CoffeeScript, which is a higher level language that provides ...


1

I think, the following line should have a else condtion to print error if it failed. if request.code == 200: From the except block, it looks like you want to print errors. You can use logging module for logging error and debug statements. If the script is intended to be used by others, you can use argparse module for command line arguments.


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