New answers tagged comparative-review
1
As we do not know the task given we cannot fully review. While the code itself looks reasonable there is some smell.
date
To me it is very smelly, that a function named id() returns a date
start_date = ids('DIR', '348')
This is also requiring a switch inside your function, so most probably there are two functions intermingled that should not be.
data ...
0
Type Hints
Use these to direct users what types of values your functions accept, and what types of values are returned.
from typing import List
def ids(a: str, b: str) -> List[str]:
...
Meaningful names
I would rename l to results, as that better demonstrates what you're returning.
You code now looks something like this:
from typing import List
...
2
There's a lot of good feedback on how to work with the data as it's structured now, but my opinion is that - as soon as humanly possible - you should deserialize it out of a collection of weakly-typed dictionaries and lists, to a set of classes. This will make a handful of things better-structured, more testable and verifiable, and more easily maintainable ...
5
Ignoring the fact that your particular code doesn't actually need a nested loop, in general I would say that nested comprehensions can be fairly readable, but helps a lot to put each for on a line of its own:
return [
some_function_of(item, product)
for item in order['order']['items']
for product in pricing['prices']
if some_condition_on(item,...
6
Readability
comprehensions have advantages
they may be short one liners that are more readable (in the context code) than explicit loops
they may be more efficient
however - when done wrong they tend to be unreadable and thus unmaintainable.
Yours is near unmaintainable. It did take me a little time to identify some of your code is superfluous. Your ...
1
On PowerShell v6+, I would do the following:
$String = $Fragment1, $Fragment2 |
Where-Object { -not [System.String]::IsNullOrWhiteSpace($_) } |
Join-String -Separator $Separator
You could use [System.String]::IsNullOrEmpty() instead of [System.String]::IsNullOrWhiteSpace(), but I find [System.String]::IsNullOrWhiteSpace() to be the desired ...
7
As a general rule it is best not to nest comprehensions. It makes code hard to read. You are better off just writing a for loop and appending the results to a list or using a generator.
Here are a couple rules with comprehensions that will make your code less brittle and easy for others to work with:
Don't nest comprehensions.
If a comprehension is too long ...
1
It depends on your needs, but generally I would say that the second approach would be better.
Why? because it allows you to run multiple tasks and that gives flexibility to your system. Now the use of synchronized may be troublesome if there are many threads running, if you don't need to handle static or shared resources/variables it would be better to not ...
0
Laravel scopes are not a way to create static methods, they should be used to add parts to a database query. They should always return a query builder.
Aim to make your scopes as small as possible so you can then combine them together to make more complex queries.
It doesn't matter if you use the Request passed in to the controller or the request() helper, ...
Top 50 recent answers are included
