Optionally running some code on a background thread Is this a good method to optionally run some code on a background thread depending on a configuration setting? This will primarily be called from an ASP.NET front-end (although others are possible) under Framework 3.5. I'm attempting to keep this as DRY as possible.

Dim sendEmails = Sub()
                     Dim emailToSend As New SendEmailRequest()
                     TransferCommonValuesTo(emailToSend, request, sendingUser)
                     usersToSendEmailTo.ForEach(Sub(u)
                                                    TransferValuesTo(emailToSend, u, m_EmailMessageMerger.GetMergedMessage(request.Message, u))
                                                    m_EmailSender.Send(emailToSend)
                                                End Sub)
                 End Sub

If cfg.SendBulkEmailUsingBackgroundThread Then
    Dim worker As New Thread(sendEmails)
    worker.IsBackground = True
    worker.Start()
Else
    sendEmails()
End If

Maybe.

Depends, as always, what happens if you get multiple threads running.

So you start this thread going, your code will move on, and if you start this again, and it hasn't finished the previous round of sending emails.   What happens?

If its all thread safe, then no problem,   but it might send some emails out of order.
If its not thread safe, then hilarity ( or calamity ) ensues :-)


Looking for a better way to define and use constants In a custom Ant task, I have the following constants:

private static final String _REVISION = ".revision";
private static final String _CODEBASE_INFO = ".codebase";
private static final String _DIFF = ".diff";
private static final String _TYPE_GIT = "git";
private static final String _TYPE_SVN = "svn";
private static final String[] _COMMAND_REV_GIT =
    {"git", "rev-parse", "HEAD"};
private static final String[] _COMMAND_REV_SVN =
    {"svn", "info"};
private static final String[][] _COMMAND_CODEBASEINFO_GIT = {
    {"git", "branch", "-v"},
    {"git", "remote", "-v"},
    {"git", "log", "--max-count=1"},
    {"git", "status"}};
private static final String[][] _COMMAND_CODEBASEINFO_SVN = {
    _COMMAND_REV_SVN,
    {"svn", "status"}};
private static final String[][] _COMMAND_DIFF_GIT = {
    {"git", "diff", "HEAD"},
    {"git", "diff", "HEAD", "--no-prefix"}};
private static final String[][] _COMMAND_DIFF_SVN = {
    {"svn", "diff"}};

private static final Map<String, Object> _COMMANDS;

static {
    Map<String, Object> map = new HashMap<>();
    map.put(_TYPE_GIT + _REVISION, _COMMAND_REV_GIT);
    map.put(_TYPE_SVN + _REVISION, _COMMAND_REV_SVN);
    map.put(_TYPE_GIT + _DIFF, _COMMAND_DIFF_GIT);
    map.put(_TYPE_SVN + _DIFF, _COMMAND_DIFF_SVN);
    map.put(_TYPE_GIT + _CODEBASE_INFO, _COMMAND_CODEBASEINFO_GIT);
    map.put(_TYPE_SVN + _CODEBASE_INFO, _COMMAND_CODEBASEINFO_SVN);
    _COMMANDS = Collections.unmodifiableMap(map);
}

What the task should do is to run commands according to a given version control type. For example if I give it git then it will run commands in _COMMAND_REV_GIT, _COMMAND_CODEBASEINFO_GIT and _COMMAND_DIFF_GIT.

The reason I separate them into three constants is that the outputs of them have to be handled in different ways.

The reason I use a map is that I don't want to use a lot of "if" or "switch-case" in my code. In the map there are keys like git.revision, git.codebase, etc. If git is given when running this task, _COMMAND_REV_GIT will be fetched from the map using the key git.revision in the method handling running the revision command:

String[] command = _COMMANDS.get(_givenType + _REVISION);

But these constants looks really complicated, and I think the types of version control should be Enum rather than String, so is the suffixes indicating the types of the commands. So how should I modify these codes to make it clear, safe and elegant?

You should create an ScmCommands interface like this:

public interface ScmCommands {
    public String[] getRevisionCommand();

    public List<String[]> getDiffCommand();

    public List<String[]> getCodebaseInfoCommand();
}

and two implementations: SvnCommands, GitCommands.

It would improve type safety a lot. (With a Map<String, Object> you usually have to cast the values which is error-prone). Furthermore, it eliminates the magic constants. It's easy to call Map.get() with a mistyped key. With the interface the compiler warns you if the method name is wrong.

Edit: I've changed the return types. Sample implementations are the following:

public class GitCommands implements ScmCommands {

    private static final String GIT_COMMAND = "git";

    public GitCommands() {
    }

    @Override
    public String[] getRevisionCommand() {
        return new String[] { GIT_COMMAND, "rev-parse", "HEAD" };
    }

    @Override
    public List<String[]> getDiffCommand() {
        final List<String[]> result = new ArrayList<String[]>();
        result.add(new String[] { GIT_COMMAND, "diff", "HEAD" });
        result.add(new String[] { GIT_COMMAND, "diff", "HEAD", "--no-prefix" });
        return result;
    }

    @Override
    public List<String[]> getCodebaseInfoCommand() {
        final List<String[]> result = new ArrayList<String[]>();
        result.add(new String[] { GIT_COMMAND, "branch", "-v" });
        result.add(new String[] { GIT_COMMAND, "remote", "-v" });
        result.add(new String[] { GIT_COMMAND, "log", "--max-count=1" });
        result.add(new String[] { GIT_COMMAND, "status" });
        return result;
    }
}

public class SvnCommands implements ScmCommands {

    private static final String SVN_COMMAND = "svn";

    public SvnCommands() {
    }

    @Override
    public String[] getRevisionCommand() {
        return new String[] { SVN_COMMAND, "info" };
    }

    @Override
    public List<String[]> getDiffCommand() {
        final List<String[]> result = new ArrayList<String[]>();
        result.add(new String[] {SVN_COMMAND, "diff"});
        return result;
    }

    @Override
    public List<String[]> getCodebaseInfoCommand() {
        final List<String[]> result = new ArrayList<String[]>();
        result.add(new String[] { SVN_COMMAND, "status" });
        return result;
    }

}

It fits well to ProcessBuilder but I'd consider extracting out all SCM related methods to an interface (for example Scm), and move the code to SCM-specific classes which implement the Scm interface, like GitScm, SubversionScm etc. Calling command line executables doesn't smell good, maybe it would be better to use native Subversion and Git Java libraries.


Check if any of the coordinates are within a certain distance using apply in Python pandas with two data frames I wrote this code that takes the vessel coordinates and checks them against every possible port coordinates. If the distance between the vessel and the port coordinates is within x meters (in this example, it has to be less than 3 km), then add that port and the distance_to_the_port to the data frame.
Example dataframes:
df_ports = pd.DataFrame({'Ports':['Port_A','Port_B','Port_C'], 'latitude': [1,5,3], 'longitude':[1,10,5]})
latitude_and_longitude_columns = df_ports.iloc[:, [1,2]]
df_ports['coordinates_tuple'] = latitude_and_longitude_columns.apply(tuple, axis=1)

df_vessel = pd.DataFrame({'latitude': [1,5,7], 'longitude':[1,10,20]})
latitude_and_longitude_columns = df_vessel.iloc[:, [0,1]]
df_vessel['coordinates_tuple'] = latitude_and_longitude_columns.apply(tuple, axis=1)

df_vessel['distance_to_port_in_meters'] = -1
df_vessel['port'] = -1

The function that checks the distance:
def get_distance_between_two_points(tuple1, tuple2):
    from geopy import distance
    radius = 3000
    dist = distance.distance(tuple1, tuple2).m
    if dist < radius:
        return dist
    return -1

The logic:
for _, port_row in df_ports.iterrows():

        port = port_row['Ports']
        result = df_vessel.apply(lambda vessel_row : get_distance_between_two_points(vessel_row['coordinates_tuple'], port_row['coordinates_tuple']), axis=1)

        for index, value in result[result != -1].items():
            df_vessel.loc[index, 'distance_to_port_in_meters'] = int(value)
            df_vessel.loc[index, 'port'] = port

Result:
 latitude  longitude coordinates_tuple  distance_to_port_in_meters    port
0         1          1            (1, 1)                           0  Port_A
1         5         10           (5, 10)                           0  Port_B
2         7         20           (7, 20)                          -1      -1

The result is correct and it works with my bigger data sample.
How can I improve this?
In my current code, I have 4800 vessel coordinates and 137 ports. It takes just above 2 minutes to go through the code. How can I make my code better and possibly faster?

You need to drop your use of geopy. So far as I can see it does not support vectorised inverse geodesics. Instead use cartopy which inherently supports this.
The following example code uses random instead of real data, but with the same size as your real data, and completes in about one second. distances and close are both matrices whose rows correspond to vessels and columns correspond to ports. close is a matrix of booleans for vessel-port pairs that are proximate.
import numpy as np
from cartopy.geodesic import Geodesic
from numpy.random import default_rng

N_VESSELS = 4_800
N_PORTS = 137

rand = default_rng(seed=0)

vessels = rand.random((N_VESSELS, 2)) + (1, -51)
ports = rand.random((N_PORTS, 2)) + (1, -51)
source = vessels.repeat(N_PORTS, axis=0)
dest = np.tile(ports, (N_VESSELS, 1))

geo = Geodesic()
distances = geo.inverse(source, dest).base[:, 0].reshape((N_VESSELS, N_PORTS))
close = distances < 3_000


Just a few things I noticed.

Utilize built in functions

This

S = 0
for k in range(1, N):
    S += function[a + k * h]

can be this

S = sum(function[a + k * h] for k in range(1, N))

Python3's sum takes an iterable, and returns the sum of all the values in that iterable. So, you can pass in the for loop and it will return the sum for you. Looks neater, too.

Operator Spacing

There should be a space before and after every operator and equal sign. Here's an example from your code:

d = 1/2 * h * (function[a + k*h] + function[a + (k+1) * h])

This should be

d = 1/2 * h * (function[a + k * h] + function[a + (k + 1) * h])

Spacing just helps you and other programmers read your code.

This, however, doesn't apply when passing default parameters. Have a look:

plot(time, distance, "g--", label = "position") # WRONG
plot(time, distance, "g--", label="position") # RIGHT

Variable Naming

A lot of single character variable names can confuse you, and make a program hard to read. I see you have comments such as #x_initial and #x_final. Why not use those as variable names? They're a lot more concise than a and b. h can also be something like diff.


Securing my password wallet I am currently learning C++, so I made this basic password wallet for Ubuntu.
Its source code is in the /opt directory and it has a  symlink so i can launch it from any directory using pw in the terminal.
Here is what it does:
It asks for a password (pass) and a site (which is related to the pass). It decrypts the file (test.enc) that contains the passwords to another file (text.txt), writes the pass and site to test.txt, encrypt it back to test.enc and delete it (test.txt).
#include <iostream>
#include <fstream>
#include <string>

using namespace std;

int main(){
        char pass[30];
        string site;
        getline(cin, site);
        cin >> pass;
        system("cd /opt/PW/ && openssl enc -aes-256-ecb -d -iter 100000 -in test.enc -out test.txt");
        ofstream MyFile("/opt/PW/test.txt", ios::app);
        MyFile << pass << ":" << site << endl;
        MyFile.close();
        system("cd /opt/PW/ && openssl enc -aes-256-ecb -iter 100000 -in test.txt -out test.enc && rm -rf test.txt");
}

It's for personal use only.
I am curious to know how it can be exploited.

Avoid calling system()
If possible, avoid calling system(). There are almost always issues doing this:

It is slow: system() will spawn another shell, which has to parse the command line you give it, and then it will spawn the actual commands you want to run).
Constructing a command line itself has all kinds of issues. For example, are you sure openssl is in your $PATH? And what if the search path finds a different version of openssl than the one you expected (/usr/bin/openssl)?
There is no easy way to send and receive data to the commands you start, so you have to redirect the input/output to files.

Especially the latter issue is important: what if someone reads /opt/PW/test.txt between the two calls to system()?
It would be much better to use a crypto library. OpenSSL is not just a program, it's also a library that you can call directly from C. However, I would say it is not the easiest to work with. There are other libraries out there that make encrypting and decrypting files easier and safer, for example NaCL.
Cryptography is hard to get right
Trying to learn about cryptography is great! However, be aware that it is not easy to get cryptography right. You are using AES-256 in ECB mode, which is bad. Furthermore, you are not using any form of message authentication, so if someone tampers with the encrypted file, then even if they cannot decrypt it properly, when you try to decrypt it you will not get the same data back as you put it. Sometimes it will be obviously garbage, but it sometimes is possible to change the data in more subtle ways (especially with ECB mode, but other modes also have issues if not combined with a MAC).
Go ahead and try to improve your program, learn more about cryptography, but I strongly suggest you do not use your cryptographic code for anything serious until you get a much better understanding of cryptography.


Efficient element-wise function computation in Python I have the following optimization problem:

  Given two np.arrays X,Y and a function K I would like to compute as fast as possible the matrix incidence gram_matrix where the (i,j)-th element is computed as K(X[i],Y[j]).

Here is an implementation using nested for-loops, which are acknowledged to be the slowest to solve these kind of problems.

def proxy_kernel(X,Y,K):
    gram_matrix = np.zeros((X.shape[0], Y.shape[0]))
    for i, x in enumerate(X):
        for j, y in enumerate(Y):
            gram_matrix[i, j] = K(x, y)
    return gram_matrix

You can avoid the nested loops using numpy.meshgrid to build a table of entries in x and y, and numpy.vectorize to apply a function to all entries in the table:

def tabulate(x, y, f):
    """Return a table of f(x, y)."""
    return np.vectorize(f)(*np.meshgrid(x, y, sparse=True))

For example:

>>> import operator
>>> tabulate(np.arange(1, 5), np.arange(1, 4), operator.mul)
array([[ 1,  2,  3,  4],
       [ 2,  4,  6,  8],
       [ 3,  6,  9, 12]])

This is about twice as fast as proxy_kernel:

>>> from timeit import timeit
>>> X = Y = np.arange(2000)
>>> timeit(lambda:proxy_kernel(X, Y, operator.mul), number=1)
2.174600816098973
>>> timeit(lambda:tabulate(X, Y, operator.mul), number=1)
0.9889162541367114

but it still has to evaluate the slow Python code in f for each entry in the table. To really speed things up you need to use whole-array operations throughout. Depending on how you've written f, it may be possible to do this directly. For example, if you have something like:

def f(x, y):
    return np.exp(-0.5 * np.abs(x - y))

then this is already suitable for applying to whole arrays:

>>> f(*np.meshgrid(np.arange(4), np.arange(4), sparse=True))
array([[ 1.        ,  0.60653066,  0.36787944,  0.22313016],
       [ 0.60653066,  1.        ,  0.60653066,  0.36787944],
       [ 0.36787944,  0.60653066,  1.        ,  0.60653066],
       [ 0.22313016,  0.36787944,  0.60653066,  1.        ]])

and this is about five times as fast as using numpy.vectorize:

>>> timeit(lambda:f(*np.meshgrid(X, Y, sparse=True)), number=1)
0.1973482659086585


Rxjs search in Angular I created an rxjs search for a hiring company, but I got rejected for the following reason:
-performance issue on a bigger data set.
-bugs
My question is how can I make this code better for the next time if I have similar task?
brands =  These should be populated from the brand property from the collection of products.

The search should match using the name, description and brand
properties.

The search should not fire unless a minimum of three characters have been typed.
The search should only fire a maximum of once per 400 milliseconds.

If searching for “xxx” with the brand selected as “xxx” and In Stock, then only products matching all three filters

const stackoverflowFilter = (filter: ISearch) => {
  return (product: IProducts): boolean => {
    return ((
      (
        (product.name.toLowerCase().indexOf(filter.input.toLowerCase()) !== -1) ||
        product.description.toLowerCase().indexOf(filter.input.toLowerCase()) !== -1 ||
        product.brand.toLowerCase().indexOf(filter.input.toLowerCase()) !== -1) &&
      product.brand.toLowerCase().indexOf(filter.select.toLowerCase()) !== -1 &&
      (filter.radio === 'all' ? true : filter.radio === 'in-stock' ? product.quantity > 0 : product.quantity === 0)
    )
    )
  };
};

export interface ISearch { input: string; select: string; radio: string }

> public getStackoverflow(): Observable<Stackoverflow[]> {
>         return of([
>           {
>             id: "1",
>             name: "This is just test data",
>             price: "89.9",
>             description: "test data",
>             brand: "stackoverflow",
>             quantity: 4
>           }....])}

 this.stackoverflow$ = this.service.getStackoverflow();

    this.stackoverflow$.pipe(
      tap((prod) => {
        this.stackoverflow = prod;
        this.brands = this.stackoverflow.map(({ brand }) => brand).reduce((prev, cur) => {
          if (prev.indexOf(cur) < 0) prev.push(cur);
          return prev;
        }, []);
      }),
      concatMap(() =>
        this.search$.pipe(
          debounceTime(400),
          distinctUntilChanged(),
          switchMap((search: string) =>
            iif(() => search.length >= 3,
              this.stackoverflow$.pipe(
                map(stackoverflow=> stackoverflow.filter(stackoverflowFilter(this.filter))),
                distinctUntilChanged((curr, prev) => curr === prev),
              ),
              this.stackoverflow$.pipe(
                filter(() => search.length === 0),
              )
            )
          )
        ))
    ).subscribe(
      stackoverflow=> { this.stackoverflow= stackoverflow; },
      (err) => throwError(err)
    );

If there are 1 million datasets, and i enter the first three letters, it will have to filter all 1 millionen values (let us asume the result will be 200 hits). When i now add a fourth letter, it will have to filter AGAIN all 1 million values and not just the 200 hits.
Therefor, if it is absolutly necessary to do the filtering codewise, then you could implement a kind of memory effect.
When the first filtering is executed (after the first 3 letters), you store the searchphrase and the filter result. When now the fourth letter is added, you just check if the first three letters are still the same. If yes, then you can shorten your filtering by just filtering the reduced list from the filtering before.
That way you swap runtime against memory consumption (a typical trade off).
My question would be if it is absolutly necessary to do the filtering codewise. When this search should be applied mostly on big datasets, then i would move the filtering to the backend. Then you have a lot more of requests to the backend, but the backend (the database to be precise) could handle such filtering much more efficiently.
But as always, it depends. For small sets of data, the roundtrip time (FE -> BE -> FE) for each search request will eat all performance gains that you have aquired with the more efficient filtering. But for large datasets, the roundtrip time will be less bad, in comparision to the filtering performance (and the memory consumption) on the client.
warm regards
Jan


Create MD5 hash list of big memory mapped files I wrote this class:

public class MediaFile : IDisposable
{

    private const int SECTION_SIZE = 8192;

    public string FilePath { get; private set; }
    public List<byte[]> Checksums { get; private set; }

    private FileInfo fileInfo;
    private MemoryMappedFile memoryMappedFile;

    public MediaFile(string filePath)
    {
        if (!File.Exists(filePath))
            throw new FileNotFoundException($"{filePath} was not found!");

        this.FilePath = filePath;
        this.Checksums = null;

        this.fileInfo = new FileInfo(filePath);
        this.memoryMappedFile = MemoryMappedFile.CreateFromFile(filePath, FileMode.Open);

        this.Checksums = this.GenerateChecksums();
    }

    public void Dispose()
    {
        this.memoryMappedFile.Dispose();
    }

    public byte[] Read(long offset, int size)
    {
        using (MemoryMappedViewStream memoryMappedViewStream = this.memoryMappedFile.CreateViewStream(offset, size, MemoryMappedFileAccess.Read))
        using (BinaryReader binaryReader = new BinaryReader(memoryMappedViewStream))
            return binaryReader.ReadBytes(size);
    }

    private List<byte[]> GenerateChecksums()
    {
        List<byte[]> checksums = new List<byte[]>();

        double sectionsCalc = this.fileInfo.Length / (double) SECTION_SIZE;

        long normalSizedSectionCount = (long) Math.Floor(sectionsCalc);
        int lastSectionSize = (int) ((sectionsCalc - normalSizedSectionCount) * (double) SECTION_SIZE);

        using (MD5 hashProvider = MD5.Create())
        {
            for (long i = 0; i < normalSizedSectionCount; i++)
                checksums.Add(hashProvider.ComputeHash(this.Read(i * SECTION_SIZE, SECTION_SIZE)));

            checksums.Add(hashProvider.ComputeHash(this.Read(normalSizedSectionCount * SECTION_SIZE, lastSectionSize)));
        }

        return checksums;
    }

}

The purpose of this class is to read large media files and create md5 hashes (checksums in this case) for all x bytes (SECTION_SIZE) of the file and add them to a list.

I want to know if there is a better/faster way to accomplish this.

You named your class MediaFile but basically its a class which creates checksums for any given file. You should consider to rename the class to e.g ChecksumCreator of FileChecksumCreator. This would make the purpose and the responsibility of that class obvious by looking at its name.
Omitting braces {} although they might be optional can lead to serious bugs which aren't tracked easily. I would like to encourage you to always use them.
You are using C# 6 but you still use private setters for your properties. With C# 6 you can just omit them if you either set the proerties at startup or from inside of the constructor. The backing field of a getter only property is internally handled as readonly and because you only set them inside the constructor you should take this route. While we are at the constructor, you are first assigning null to the Checksums property and later on you are using the GenerateChecksums() method to assign a value to it. The mentioned points could be implemented like so

public string FilePath { get; }
public List<byte[]> Checksums { get; }

private FileInfo fileInfo;
private MemoryMappedFile memoryMappedFile;

public MediaFile(string filePath)
{
    if (!File.Exists(filePath))
    {
        throw new FileNotFoundException($"{filePath} was not found!");
    }

    this.FilePath = filePath;

    this.fileInfo = new FileInfo(filePath);
    this.memoryMappedFile = MemoryMappedFile.CreateFromFile(filePath, FileMode.Open);

    this.Checksums = this.GenerateChecksums();
}

But basically, you won't need a FileInfo object here, because you are only using the Length property of the FileInfo object. So let us change this like so

public string FilePath { get; }
public List<byte[]> Checksums { get; }

private long fileLength;
private MemoryMappedFile memoryMappedFile;

public MediaFile(string filePath)
{
    if (!File.Exists(filePath))
    {
        throw new FileNotFoundException($"{filePath} was not found!");
    }

    this.FilePath = filePath;

    this.fileLength= new FileInfo(filePath).Length;
    this.memoryMappedFile = MemoryMappedFile.CreateFromFile(filePath, FileMode.Open);

    this.Checksums = this.GenerateChecksums();
}

But what happens if you decide later on to go the way @EricLippert has shown in the comments, like using a CRC hashprovider to generate the checksums ? You would need to change the class in question. Maybe it would be better to inject a HashAlgorithm provider so you could easily switch to another one.

This would result in an overloaded constructor and a slightly changed Dispose() method like so

private bool hasOwnershipOfHashprovider = false;
private HashAlgorithm hashProvider;

public MediaFile(string filePath)
    : this(filePath, MD5.Create())
{
    hasOwnershipOfHashprovider = true;
}
public MediaFile(string filePath, HashAlgorithm hashProvider)
{
    if (!File.Exists(filePath))
    {
        throw new FileNotFoundException($"{filePath} was not found!");
    }
    if (hashProvider == null)
    {
        throw new ArgumentNullException($"hashProvider");
    }

    this.FilePath = filePath;

    this.hashProvider = hashProvider;
    this.fileLength = new FileInfo(filePath).Length;
    this.memoryMappedFile = MemoryMappedFile.CreateFromFile(filePath, FileMode.Open);

    this.Checksums = this.GenerateChecksums();
}

public void Dispose()
{
    this.memoryMappedFile.Dispose();
    if (this.hasOwnershipOfHashprovider)
    {
        this.hashProvider.Dispose();
    }
}

But what happens if the user of that class won't call Dispose() ? Take a look at proper-use-of-the-idisposable-interface.
I don't see why the Read() method should be public. If it needs to be public, you should add some proper validation of the passed arguments.
If a filePath is passed to the constructor which represents a file with Length == 0 the MemoryMappedFikle.Create() method will throw an ArgumentException which should better be thrown by your code.


Calculate the value of an arithmetic term written in Polish Notation I have to create a program which calculates the result of an arithmetic term written in Polish Notation.

var parseNotation = parseNotation || {};

/**
 * Computes the result of an given arithmetic term which is written in
 *  Reverse Polish Notation (https://en.wikipedia.org/wiki/Reverse_Polish_notation).
 * @param { String } termInReversePolish - The operators / operands of
 *  the term have to be separated by colons. See usage example!
 * @return { number } Result of the given term.
 * @throws { Error } Will throw an error in case of no or invalid parameter
 *  is given.
 *
 * Usage example (will display 37 as result):
 *  console.log(parseNotation.computeFromReversePolish('5,6,2,+,*,12,4,/,-'));
 */

parseNotation.computeFromReversePolish = function(termInReversePolish) {
  var currentChar = '';
  var stack = [];
  var right = 0;

  if (typeof termInReversePolish !== 'string') {
    throw new Error(
      `No valid parameter given. String expected but ${typeof termInReversePolish} found.`);
  }

  if (termInReversePolish.search(/[^\d,+*\/-]/g) != -1) {
      throw new Error(
        'Given parameter contains illegal characters. Valid characters are: + - * / , 0-9');
  }

  termInReversePolish = termInReversePolish.split(',');

  termInReversePolish.forEach((currentChar, i) => {
    if (['+', '-', '*', '/'].indexOf(currentChar) === -1) {
      stack.push(currentChar);
    } else {
      right = stack.pop();

      stack.push(eval(stack.pop() + currentChar + right));
    }
  });

  return stack.pop();
}

// ---- Testing ------------------
console.log(parseNotation.computeFromReversePolish( '1,2,+,3,4,+,*' ));
console.log(parseNotation.computeFromReversePolish( '5,6,2,+,*,12,4,/,-' ));
console.log(parseNotation.computeFromReversePolish( '2,7,+3,/,14,3,-,4,*,+,2,/' ));

My first few test with the function have gone well. Nevertheless, I would like to read the opinion of other developer, especially regarding:

Is my documentation done in a proper way? Or does I have to improve it?
Is my parameter validation sufficient? Especially regarding the used regular expression.
Generally: Is everything done in a good way and fashion?

If someone sees an error or flaw then please let me know.

To start at the end:

  Generally: Is everything done in a good way and fashion?

It looks quite good, kudos!

Structurally, there's one thing that's not quite right, though: You're declaring currentChar and right in the function's scope, you really don't need to. The currentChar you've declared never gets used; the argument in the forEach callback shadows it (and it's not necessarily a character - it could be "12312"). And right doesn't have to be in that scope either, it can just be local to the callback.

And you could technically use reduce instead of forEach with a closed-over stack, but that's not so bad.

I also have some more notes, but also I'm nit-picking quite a bit. I had to find something to write, y'know?

Your documentation seems fine. The exact flavor of documentation syntax is more or less up to you (JS doesn't have a strong convention), so the JavaDoc-like syntax is fine. Would also have accepted a regular comment without the @-keywords, so long as the important parts are covered.

You might consider some comments in the function body, though. There isn't much need for it here admittedly, but just to say that it's not just about commenting the API but also the code.

The regex validation seems fine too, but it doesn't catch malformed syntax, only illegal tokens. A string like ",,," wouldn't trigger anything, nor would a string like "3,+".

The latter ("3,+") returns NaN, which is sort of OK. You can evoke the rule of "Garbage in, garbage out" and blame the caller. At least they're getting a Number back (yes, NaN - not a number - is in fact a Number. Go figure.)

The former input, however, returns an empty string. Since no operations have been performed, the final stack.pop() will be an empty string. So here, you're not even getting a number. That seems less correct.

Ideally, you would complain if the function reaches the end with something other than 1 element in the stack, since that would imply that the RPN expression wasn't "balanced". In fact, your third example suffers from this; the stack ends at [2, 23.1666...], but you report the 23.166... as the answer despite the expression not being fully parsed.

It would also be nice to allow (and ignore) whitespace in the string, just to make input a little flexible. A more readable input string might be beneficial to users (I tested your code, and kept putting in spaces after commas out of habit).

There's another thing your code doesn't handle: Anything with decimals. Since you can't use . in the string, you're limited to integers. That's a bit harsh. You could also consider allowing other legal JS number formats like 0x3 hex, 0b1010 binary, or 3.14e3, but that's more just an exercise for you. eval should already handle them fine, but your regex will complain.

Last bit about validation: I'd throw a TypeError if the input isn't a string. More accurate than a generic Error certainly.

That's mostly smaller stuff, and easily fixed or left as-is. The only thing I'd do very differently would be to not use eval. If you instead define the operators as functions, you:

get rid of eval, which is always nicer in my opinion
you can implement operators that don't take 2 operands such as factorial
you can easily implement more operators (right now, you'll have to change both the regex and the array in the loop)

And if you store your functions in an object, you get quicker lookup than with indexOf (not that that really matters that much).

For instance:

function rpn(expression) {
  // define our operators as functions
  const operators = {
    '+': (a, b) => a + b,
    '-': (a, b) => a - b,
    '*': (a, b) => a * b,
    '/': (a, b) => a / b,
    '^': Math.pow
  };

  // check input type
  if (typeof expression !== 'string') {
    throw new TypeError(`No valid parameter given. Expected string but got ${typeof expression}.`);
  }

  // evaluate the expression, ignoring whitespace
  var stack = expression.replace(/\s/g, '').split(',').reduce((stack, term) => {
    if (operators.hasOwnProperty(term)) {
      const op = operators[term];

      // check stack depth
      if (stack.length < op.length) {
        throw new Error("Stack underflow");
      }

      // grab n elements from the stack, where n is the number of arguments
      // that the operator function takes
      var operands = stack.splice(-op.length);

      // perform the operation and push the result
      stack.push(op.apply(null, operands));
    } else {
      var number = term * 1; // coerce the term to a number

      // check that the coercion produced something numeric
      if (Number.isNaN(number)) {
        throw new Error(`The term '${term}' is neither numeric or a recognized operator`);
      }

      stack.push(number);
    }

    return stack;
  }, []);

  if (stack.length !== 1) {
    throw new Error("Incomplete expression"); // find something better to say
  }

  return stack[0];
}


Is this correct to reassign already intialized variable? I am reading hadoop source code , Here I have a doubt "org.apache.hadoop.util.StringUtils" they reassigned a list which is already intiailized.

  public static Collection<String> getStringCollection(String str){
    List<String> values = new ArrayList<String>();
    if (str == null)
      return values;
    StringTokenizer tokenizer = new StringTokenizer (str,",");
    values = new ArrayList<String>();
    while (tokenizer.hasMoreTokens()) {
      values.add(tokenizer.nextToken());
    }
    return values;
  }

Is there any reason to reassign here?

It looks like a useless reassignment.  It must be some refactoring gone wrong.  At worst, it will cause a tiny performance hit.


find if a gtin is valid I was given that assignment recently as part of a coding interview.

The idea is to find if a gtin is valid or not valid.

The requirements are the following:

make sure the input is in a string format
the length must be either 8,12,13,14,17 or 18

The algorithm is the following:

TL;DR: You need to multiply all odd positions by 3 and you have to remove the last position. For example if the GTIN is 13, you need to multiply the 12th position by 3, the 11th position by 1, the 12th position by 3 and so on.

If the GTIN is 8, you need to multiply the 7th position by 3, the 6th by 1, the 5th by 3 and so on.

Once done, you sum all the elements and you find the nearest equal or higher multiple of ten and you substract this from the sum you have obtained. The integer that you obtained should be equal to the last digit (13th position if the GTIN is 13).

If it is not, then the GTIN is an incorrect one.

Here is my code

 def gtin_check(gtin: str)-> str:
    if not isinstance(gtin, str) or len(gtin) not in (8,12,13,14,17,18):
        return 'Not a string'
    else:
        gtin_lst = [int(x) for x in gtin]
        #print("step 1 - give the gtin but as a list of integers",gtin_lst)
        gtin_lst.reverse()
        #print("step 2 reverse the gtin",gtin_lst)
        gtin_original = gtin_lst #saving the list to gtin_original
        #print("step 3 : gtin original still reverse",gtin_original)
        gtin_lst = gtin_original[1:]
        #print("step 4 - gtin_lst starting from the first element",gtin_lst)
        gtin_original.reverse()
        #print("step 5 - gtin original without reverse , going back to normal ",gtin_original)

        # arriving at the algorithm part
        gtin_lst = [x*3 if i%2 == 0 else x for i,x in enumerate(gtin_lst)]
        #print('step 6 - multiplying by 3 of integers on odd position',gtin_lst)
        som_gtin1 = sum(gtin_lst)
        #print('step 7 - gtins summation',som_gtin1)

        if (som_gtin1 % 10):
            som_gtin2 = som_gtin1 + (10 - som_gtin1 % 10) # (10 // gcd(som_gtin1, 10)) *  som_gtin1 #int(round(som_gtin1, -1)) #som_gtin1 + (10 - som_gtin1 % 10)
        else:
            som_gtin2 = som_gtin1
        #print('step 8 : multiple of 10 instead of equal or higher summation',som_gtin2-som_gtin1)

        if (som_gtin2 - som_gtin1) == gtin_original[-1]:
            return True
        else:
            return False

The code does what I want but I feel it is not pythonic enough. Any suggestions or insight on how can I write a lighter code are welcomed.

update 1:

Example 12345670 will put True. 12345678 will put False.

update 2:

Since MJ713's feedback, I've commented my code just to make sure you understand my code. Let me know if you have any issues in reading it. I've amended my code to fix the bugs, MJ713 mentionned.

I'll start from the top and work my way down to the bottom.

 def gtin_check(gtin: str)-> str:

Your type-hinting is incorrect here. Except in cases of invalid input, you are returning a boolean, not a string. Which leads me to...
    if not isinstance(gtin, str) or len(gtin) not in (8,12,13,14,17,18):
        return 'Not a string'

In general it is a bad idea to mix two completely different return types for one function, unless there is some pressing reason to do so. The mixing creates more work for whoever is calling your function, since they have to handle both types. (Null value/None is probably the most common exception to this rule; sometimes it's appropriate to return None, e.g. if a function that is searching for a single specific object or value doesn't find one.)
To get rid of the string return value, I would suggest one of these alternatives:

If the calling function really needs to know the difference between an input that had a bad check digit, and an input that was invalid in a "stronger" way: either raise an exception, or make your return type an enum with three possible values (e.g. VALID, INVALID, and MALFORMED).
If the calling function only needs to know whether an input was a valid GTIN or not: just return False.

    if not isinstance(gtin, str) or len(gtin) not in (8,12,13,14,17,18):
        return False

Now let's talk about performance. in / not in checks are generally faster on sets than they are on lists or tuples. This is because sets do clever things with hashing (\$O(1)\$), while with lists and tuples you have to go down the line and check every value in order (\$O(n)\$). So let's use a set instead.
    if not isinstance(gtin, str) or len(gtin) not in {8,12,13,14,17,18}:
        return False

While we're doing these other checks, it would also be good to use .isdecimal() to check that the input string is numeric. Right now we don't have any code to catch non-numeric inputs like 'asdfghjk', so we get a slightly ugly error (ValueError: invalid literal for int() with base 10: 'a').
    if (not isinstance(gtin, str)
            or not gtin.isdecimal()
            or len(gtin) not in {8,12,13,14,17,18}):
        return False

The line was getting a bit long, so I added some line breaks. See the PEP 8 document for guidelines on when and how to use line breaks.

        gtin_lst = [int(x) for x in gtin]
        #print("step 1 - give the gtin but as a list of integers",gtin_lst)
        gtin_lst.reverse()
        #print("step 2 reverse the gtin",gtin_lst)
        gtin_original = gtin_lst #saving the list to gtin_original
        #print("step 3 : gtin original still reverse",gtin_original)
        gtin_lst = gtin_original[1:]
        #print("step 4 - gtin_lst starting from the first element",gtin_lst)
        gtin_original.reverse()

This section is confusing - you're putting the original value in a new variable and the altered value in the original variable, not to mention reversing a reversal. It would be easier to understand if you left gtin_lst intact and used negative indexes to extract the subset you need:
        gtin_lst = [int(x) for x in gtin]
        gtin_lst_without_check_digit = gtin_lst[:-1]
        gtin_lst_without_check_digit.reverse()

        # arriving at the algorithm part
        gtin_lst = [x*3 if i%2 == 0 else x for i,x in enumerate(gtin_lst)]

It is more "pythonic" to avoid == 0 in conditions, and instead use the inherent "falsiness" of 0.
x*3 if not i%2 else x

I see you have already done this in another part of the code: if (som_gtin1 % 10):.

        if (som_gtin1 % 10):
            som_gtin2 = som_gtin1 + (10 - som_gtin1 % 10) # (10 // gcd(som_gtin1, 10)) *  som_gtin1 #int(round(som_gtin1, -1)) #som_gtin1 + (10 - som_gtin1 % 10)
        else:
            som_gtin2 = som_gtin1

The logic here is solid, but this seems like a good place to talk about naming.
In general, naming variables by the pattern foo1, foo2, etc. is a code smell. If you have more than one of the same kind of thing, why aren't they in some kind of list or collection? And if
they aren't the same kind of thing, why do they have the same name? There are exceptions, but I would say this is a bad idea nine times out of ten.
In this case, som_gtin1 and som_gtin2 aren't the same kind of thing.

som_gtin1 is a sum of some numbers. (Is "som" instead of "sum" a typo, or are you deliberately using a non-English word?)
som_gtin2 is som_gtin1 rounded to the nearest highest 10. In other words, it is based on a sum, but it is not a sum itself.

Also, since the function is named gtin_check, we already know that everything we're doing is related to GTINs somehow. So adding _gtin to the variable names doesn't really give the reader any additional information.
Picking accurate and descriptive names throughout your code will make it easier to read and understand. You will sometimes hear people describe this kind of code as "self-documenting", because it does not need nearly as many comments or external documents.
        if (digits_sum % 10):
            uprounded_sum = digits_sum + (10 - digits_sum % 10)
        else:
            uprounded_sum = digits_sum

Let me be clear: I'm not claiming that my names are perfect either. There's an old joke that "there are only two hard problems in computer science: cache invalidation and naming things."

        if (som_gtin2 - som_gtin1) == gtin_original[-1]:
            return True
        else:
            return False

I had a computer science professor in college who specifically warned us not to use this construction. So naturally, I didn't notice it until an hour after I finished writing the rest of this answer.
As my professor pointed out, any logic of the form "if boolean value, return true, else return false" can be reduced to "return boolean value".
        return ((som_gtin2 - som_gtin1) == gtin_original[-1])

The wrapping parentheses aren't strictly necessary, but I think they make the code clearer.

Putting it all together
def gtin_check(gtin: str) -> bool:
    """Returns whether the input is a well-formed GTIN string."""
    # Check for obvious problems
    if (not isinstance(gtin, str)
            or not gtin.isdecimal()
            or len(gtin) not in (8,12,13,14,17,18)):
        return False
    else:
        # Compute correct "check digit" and compare to input's digit
        original_digits = [int(x) for x in gtin]
        digits_without_check_digit = original_digits[:-1]

        digits_without_check_digit.reverse()
        multiplied_digits = [x*3 if not i%2 else x
                             for i,x
                             in enumerate(digits_without_check_digit)]
        digits_sum = sum(multiplied_digits)

        if (digits_sum % 10):
            uprounded_sum = digits_sum + (10 - digits_sum % 10)
        else:
            uprounded_sum = digits_sum
        expected_check_digit = uprounded_sum - digits_sum

        return (original_digits[-1] == expected_check_digit)


Random number generator [Beginner] I'm learning Python and I'm building a basic random numbr generator. The code below works as expected. I'm just wondering if it uses best practice / can be made better.
Some future considerations might be adding a way for users to pick a random that isn't limited to 0-50 or 0-100, and finding a way to scale the number of guesses dynamically based on their choice.
import random

"""
Make a program in which the computer randomly chooses a number between 1 to 10,
1 to 100, or any range.
"""

def intro():
    print(f"Welcome to our number guessing game.")
    print(f"We'll ask you to pick a range of numbers from 0 up to 100."
          f"Then you'll get some chances to guess the number. "
          f"The number of guesses you get will scale based on the range you "
          f"choose. From 0 - 50, you'll get 5 chances. From 0 - 100, you'll"
          f"get 10 chances. Good luck!")
    print("Choose a range: ")
    print("1. 0 - 50")
    print("2. 0 - 100\n")
    num_guesses = {
        "1": (5, 50),
        "2": (10, 100)
    }
    num_range = input()
    while True:
        if num_range == "1" or num_range == "2":
            return num_guesses[num_range]
        else:
            print(f"{num_range} is not a valid selection. "
                  f"Please select either 1 or 2.")
            num_range = input()

def guess_number(num_guesses, num_range):
    random_number = random.randint(0, num_range)
    guess_count = 0
    print(f"Random num is: {random_number}")
    while guess_count < num_guesses:
        guess = int(input(f"Pick a random number between 0 and "
                          f"{num_range}: \n"))
        if guess == random_number:
            print("CONGRATS, YOU WIN!")
            break
        else:
            guess_count += 1
            print(f"Incorrect, try again! You have "
                  f"{num_guesses - guess_count} guess(es) left.\n")
    else:
        print("Womp womp, you lose :(.")

if __name__ == "__main__":
    num_guesses, num_range = intro()
    guess_number(num_guesses, num_range)

First, an issue with behaviour: In guess_number you don't make sure the input from the user is a number at all. So you might end up calling int("oops"). This raises a ValueError, and since there's nothing to catch that, the program crashes. You'll want to do something about that. A simple solution could be
try:
    guess = int(input(...))
    if guess == random_number:
        ...
    else
        ...
except ValueError:
    print("That doesn't look like a number to me, but that's OK, I'll count it anyway :)")
    guess_count += 1

Side note, guesses that aren't in the interval also add to the counter. That might be on purpose, but since the validity of the input wasn't checked at all I don't want to assume, so I'm pointing it out.
There are also a couple minor issues with intro
First off, you're duplicating the num_range = input() line. If you get the input at the start of the loop (before the if) instead it'll be easier to change if for some reason you need to
Second, in python the usual approach is that it's better to ask for forgiveness than permission. That if check would usually be written using a try/except KeyError block (a bit like above). But if you don't want to do that, a more robust approach than the current one could be to check if num_range in num_guesses, in case a new option is added, or the program changes from accepting 1 and 2 to accepting "a" and "b".
On a related third note, the messages don't actually know what options are available, so there's a risk they get out of sync with the rest of the program. You can get a list of valid options as num_guesses.keys(), or use a for loop to iterate over the keys in num_guesses directly
Finally, I don't love the variable names. num_guesses sounds like it should be the number of guesses, but it isn't a number at all. Similarly, num_range doesn't have much to do with ranges of numbers either. num_range feels like it would be better described by a name like selection or choice, and num_guesses is a collection of available options, so maybe it could be called options?
options = {
    "1": (5, 50),
    "2": (10, 100)
}

print("Welcome to ... on the range you choose.")

for option in options:
    print(f"From 0 - {options[option][1]}, you'll get {options[option][0]} chances.")

while True:
    print("Choose a range:")
    for option in options:
        print(f"{option}. 0 - {options[option][1]}")

    selection = input()
    try:
        return options[selection]
    except KeyError:
        print(f"{selection} is not a valid selection.")


Check the response of an API for correctness I expecting the response of an API to be a array of objects like this:

[
    {
        "code": "X",
        "name": "Product A",
        "pricings": []
    },
    {
        "code": "Y",
        "name": "Product B",
        "pricings": []
    },
    {
        "code": "Z",
        "name": "Product C",
        "pricings": []
    }
]

I want to check if the response actually looks like this, to sort out error's.

I did the following:

function checkResponseApi(response, mustHave) {
    if(Array.isArray(response)) {

        if(response.length === 0) {
            return false;
        }

        var success = response.every(obj => {
            var keys = Object.keys(obj);
            var output = mustHave.map(el => keys.includes(el));

            return output.indexOf(false) === -1;
        })

        return success;
    } else {
        return false;
    }
}

if (checkResponseApi(arr, ["code", "name", "pricings"])) {
    // continue
} else {
    // handle error
}

Is this a valid solution? Maybe you can share some experience and provide another/better approach.

Thanks in advance.

Towards better performance and code organization

The list of required fields is better defined as const at top level:

const requiredFields = ["code", "name", "pricings"];

There are 2 negative outcomes that can be combined and checked at once:
1) if the response is not of Array type, 2) if it's empty array. The Consolidate conditional expression technique is applied.

if (!Array.isArray(resp) || !resp.length) {
    return false;
}

Instead of getting list of keys for each object in response array with var keys = Object.keys(obj) - the more efficient and faster way is to just apply obj.hasOwnProperty(el) to check if an object obj has required field el as a key.
All the rest "movements" in the initial approach are redundant and eliminated.

The final optimized approach:

function checkResponseApi(resp, mustHave) {
    if (!Array.isArray(resp) || !resp.length) {
     return false;
    }
    return resp.every(obj => {
        return mustHave.every(el => obj.hasOwnProperty(el))
    });
}

let arr = [
    {"code": "X", "name": "Product A", "pricings": []},
    {"code": "Y", "name": "Product B", "pricings": []},
    {"code": "Z", "name": "Product C", "pricings": []}
];
const requiredFields = ["code", "name", "pricings"];

if (checkResponseApi(arr, requiredFields)) {
    console.log('Ok')
} else {
    console.log('Bad response')
}


Baconian Cipher Python Implementation I have been trying to implement various substitution ciphers in python. I implemented the Baconian cipher using a dictionary in which the plaintext letters are keys and the ciphertext letters are the corresponding values. Is there a way to make the following code shorter or more efficient or is there a better approach to implementing the cipher.
Python program to implement Baconian cipher
'''This script uses a dictionary instead of 'chr()' & 'ord()' function'''

'''
Dictionary to map plaintext with ciphertext
(key:value) => (plaintext:ciphertext)
This script uses the 26 letter baconian cipher
in which I,J & U,V have distinct patterns
'''
lookup = {'A':'aaaaa', 'B':'aaaab', 'C':'aaaba', 'D':'aaabb','E':'aabaa',
    'F':'aabab', 'G':'aabba', 'H':'aabbb', 'I':'abaaa', 'J':'abaab',
    'K':'abaab', 'L':'ababa', 'M':'ababb', 'N':'abbaa', 'O':'abbab',
    'P':'abbba', 'Q':'abbbb', 'R':'baaaa', 'S':'baaab', 'T':'baaba',
    'U':'babaa', 'V':'babab', 'W':'babaa', 'X':'babab', 'Y':'babba'
    'Z':'babbb'}
#Function to encrypt the string according to the cipher provided
def encrypt(message):
    cipher = ''
    for letter in message:
        #checks for space
        if(letter != ' '):
            #adds the ciphertext corresponding to the plaintext from the dictionary
            cipher += lookup[letter]
        else:
            #adds space
            cipher += ' '

    return cipher

#Function to decrypt the string according to the cipher provided
def decrypt(message):
    decipher = ''
    i = 0

    #emulating a do-while loop
    while True :
        #condition to run decryption till the last set of ciphertext
        if(i < len(message)-4):
            #extracting a set of ciphertext from the message
            substr = message[i:i+5]
            #checking for space as the first character of the substring
            if(substr[0] != ' '):
                '''
                This statement gets us the key(plaintext) using the values(ciphertext)
                Just the reverse of what we were doing in encrypt function
                '''
                decipher += list(lookup.keys())[list(lookup.values()).index(substr)]
                i += 5 #to get the next set of ciphertext
            else:
                #adds space
                decipher += ' '
                i += 1 #index next to the space
        else:
            break #emulating a do-while loop

    return decipher

def main():
    message = "ALICE KILLED BOB"
    result = encrypt(message.upper())
    print (result)

    message = "aaaaaababaabaaaaaabaaabaa abaababaaaababaababaaabaaaaabb aaaababbabaaaab"
    result = decrypt(message.lower())
    print (result)

#Executes the main function
if __name__ == '__main__':
    main()

I have been implementing most of the substitution ciphers using dictionaries and I want to know is there a another approach to this as I am bored of using dictionaries.

Bug
Letters J & K, U & W, and V & X have the same pattern. This is not what the specification says: either I & J and U & V shares the same pattern, or none of the letters do.
Style
String literals ain't comments. If you want two lines of comments, you start your two lines with a # each. Also take the habit to put a space after the # sign, it eases readability.
You also don't need to put parentheses around simple conditions.
Lastly, PEP 8 recommends to use ALL_CAPS to name constants, like LOOKUP.
Generators
Concatenating strings using += is slow as strings are immutables and Python need to copy the whole string each time you add a bit at the end. For better performances, it is advised to use ''.join. To do this, it is easier to separate each function into two: one that iterates over the input and joins the converted output, the other that generates the converted output. Something like:
def encrypt_letter(letter):
    """Convert a plain-text ascii letter into its Bacon's form.

    Let symbols and non ascii letter fail through gracefully.
    """
    return LOOKUP.get(letter.upper(), letter.lower())

def encrypt(message):
    """Encrypt a message using Bacon's cipher"""
    return ''.join(map(encrypt_letter, message))

One more thing to note with this implementation: you shouldn't require that the input is properly formatted, you should enforce it in your functions (usage of upper and lower within your function, not before its call)
Do-while
In Python, a do-while loop is emulated using something along the lines of:
while True:
    do_something()
    if ...:
        break

In your case, having the if directly as the first action of the while and an else: break, you just need a simple while loop:
while i < len(message) - 4:
    substr = message[i:i + 5]
    ...


Pure JS multi-slide carousel On our home page we need simple multi-slide carousel. It is our highest traffic page, so we decided to remove all libraries (including jQuery) in order to increase page speed.

Functionality we trying to achieve is similar to this

Here is the code we have now: Codepen

function getStyleValue(elem, property) {
  return window.getComputedStyle(elem).getPropertyValue(property);
}

const arrows = document.querySelectorAll(".carousel .arrow"),
  [leftArrow, rightArrow] = arrows,
  slideContainer = document.querySelector(".carousel .slide-container"),
  slides = document.getElementsByClassName("slide"),
  amt = parseInt(getStyleValue(slides[0], "width"));

try {
  let trans = parseInt(
    getStyleValue(slideContainer, "transform")
      .split(",")[4]
      .trim()
  );
} catch (e) {
  console.log("`trans` being set to 0");
  trans = 0;
}

function slide() {
  if (this === rightArrow) {
    trans -= amt;
  } else if (this === leftArrow) {
    trans += amt;
  }
  if (trans <= amt * slides.length / 2 * -1) {
    rightArrow.style.display = "none";
  } else if (trans >= 0) {
    leftArrow.style.display = "none";
  } else {
    arrows.forEach(arrow => (arrow.style.display = "inline-block"));
  }
  slideContainer.style.transform = `translate(${trans}px)`;
}

arrows.forEach(arrow => arrow.addEventListener("click", slide));
body *::selection {
  background: none;
}

.carousel {
  --height: 128px;
  --width: 120px;
  display: block;
  margin: 100px auto 0;
  height: var(--height);
  text-align: center;
}

.carousel * {
  display: inline-block;
  position: relative;
  vertical-align: top;
  height: 100%;
}

.arrow-container {
  --width-arrow: 64px;
  width: var(--width-arrow);
}

.arrow {
  --arrow-width: calc(var(--width-arrow) / 2);
  position: absolute;
  top: 50%;
  left: 50%;
  width: var(--arrow-width);
  height: var(--arrow-width);
  background-color: white;
  font-weight: bold;
  line-height: var(--arrow-width);
  border-radius: 50%;
  transform: translate(-50%) translateY(-50%);
  transition: background-color 111ms, color 222ms;
  cursor: pointer;
}

.arrow:hover {
  background-color: rgba(0, 0, 0, 0.2);
  color: white;
}

.arrow:active {
  transform: scale(0.9) translate(-50%) translateY(-50%);
}

.arrow.left {
  display: none;
}

.display-container {
  /* * 4 to show 4 slides */
  width: calc(var(--width) * 4 - 2px);
  overflow-x: hidden;
}

.slide-container {
  /* * 8 cause that's the number of slides */
  width: calc(var(--width) * 8);
  /* start by going left 2 slides */
  /* transform: translate(calc(var(--width) * -2)); */
  transition: transform 333ms;
}

.slide {
  margin-left: -4px;
  width: var(--width);
  background-color: salmon;
}

.slide:nth-child(2n) {
  background-color: cornflowerblue;
}

    &lt;

1

2

3

4

5

6

7

8

    &gt;

Minor issues

Don't use window. inconsistently. You use it in some places and then not in others. It's just noise so the best option is that if you find yourself typing window.blah you have added 7 needless characters.
Don't use compound declarations, they can be a source of hard to spot errors, especially as you are not using strict mode.
"use strict" Never release code that does not use strict mode, and it must have used strict mode from day one.
Avoid using parseInt unless you define the radix, eg parseInt("16",10). Better way to parse a string to a number is via Number(string). Or a safer form const number = isNaN(str) ? defaultNumber : Number(str);
Your slide function at first tests for both the left and right arrow. but if the only events that will generate the call are the two arrows the second statement should be else and not include the redundant if statement.
Don't do complex expressions inside conditional statements. Generally conditional statement should be limited to the test. You had if (trans <= amt * slides.length / 2 * -1) { would be better split as two lines; const pos =amt * slides.length / 2 * -1; then if(trans <= pos) {

Bad code.

There are some that will not agree, this is an opinion on good coding practice for JavaScript.

The code in question

try {
  let trans = parseInt(
    getStyleValue(slideContainer, "transform")
      .split(",")[4]
      .trim()
  );
} catch (e) {
  console.log("`trans` being set to 0");
  trans = 0;
}

There are very few situations in javascript when you need to use a try catch block.  I feel that if you are using them you are being lazy.

There are many problems that can be overlooked when using try catch. Here are just 3 for your case.

You catch all errors, not just the one you have planned for. Code running in the catch should always check the error and only handle the one you intend to handle, all other errors should be thrown to a higher level.

As you have it you will consume all errors, errors that could cause problems later in your code but will not know about till way after it first threw an error.
You are forcing the error via the trim function TypeError: Cannot read property 'trim' of undefined this is no guarantee of the problem being caught.

I was at first puzzled as to why the trim was there as parseInt will do it for you, till it dawned on me, to throw the (incorrect) error.

So not only badly using the try catch you are not using a standard method to throw a known error. You should have used throw and thrown a new Error("blah blah"); (I put "blah blah" because I do not consider it an error at all and have no words to describe it.)
Major scope problem You will never get the correct result as trans is declared as scoped to the try block and the catch if invoked assigns to a new global scope trans.

The only value trans will have after that code has run is as an undefined undeclared variable (Very dangerous) or as the global scoped trans = 0;

It does not take much effort to avoid the try catch, and in this case the  cluster of problems it has set up in your code.

Quick rewrite.

I did not look at variable naming, function design and layout, and other issues as It was the try catch the got my attention.

Thus all I did was rewrite the try catch block as a function returning transform property or zero. If you run it here as snippet, it will throw for a very different reason though.

function getTrans(element){
  var value = getStyleValue(element, "transform");
  value = value.split(",")[4];
  if(value !== undefined){
    return Number(value);
  }
  return 0;
}
var trans = getTrans(slideContainer);


Finding points with the shortest distance This takes in a user specified number of points then finds the two points with the shortest distance.

import java.util.Scanner;
import java.lang.Math;

public class FindNearestPoints
{
    public static void main(String[] args)
    {
        Scanner scan = new Scanner(System.in);

        System.out.print("Enter the number of points: ");
        final int numOfPoints = scan.nextInt();

        double[][] points = new double[numOfPoints][2];
        double shortestDistance=0;
        double distance=0;
        String closestPoint1="";
        String closestPoint2="";

        //enter x,y coords into the ix2 table points[][]
        for (int i=0; i<numOfPoints; i++)
        {
                System.out.print("Enter point x" + i + ": ");
                points[i][0] = scan.nextDouble();
                System.out.print("Enter point y" + i + ": ");
                points[i][1] = scan.nextDouble();
        }

        //get the distance between the point in the ith row and the (m+1)th row
        //and check if it's shorter than the distance between 0th and 1st
        for (int i=0; i<numOfPoints; i++)
        {
            //use m=i rather than 0 to avoid duplicate computations
            for (int m=i; m<numOfPoints-1;m++)
            {
                double dx = points[i][0] - points[m+1][0];
                double dy = points[i][1] - points[m+1][1];
                distance = Math.sqrt(dx*dx + dy*dy);

                //set shortestDistance and closestPoints to the first iteration
                if (m == 0 && i == 0)
                {
                    shortestDistance = distance;
                    closestPoint1 = "(" + points[0][0] + "," + points[0][1] + ")";
                    closestPoint2 = "(" + points[1][0] + "," + points[1][1] + ")";
                }
                //then check if any further iterations have shorter distances
                else if (distance < shortestDistance)
                {
                    shortestDistance = distance;
                    closestPoint1 = "(" + points[i][0] + "," + points[i][1] + ")";
                    closestPoint2 = "(" + points[m+1][0] + "," + points[m+1][1] + ")";
                }
            }
        }
        System.out.println("The shortest distance is: " + shortestDistance);
        System.out.println("The closest points are " + closestPoint1 + " and " + closestPoint2);
    }
}

For starters, this code should use a Point class, consisting of an x and y location. Your variable points would then be Point[] rather than a two dimensional array. The Point class should have a method for reading values, printing values and calculating the distance between two points.

Then, using the Point class, your code would be dramatically simpler and easier to read. It might even be easier to write from scratch rather than try and fix what you have.


How can I improve my A* algorithm in Python? I'm trying to implement my own A* pathfinder, but the voices in my head scream of better ways to do this.  Probably because I'm exceeding Python's default maximum recursion depth of 1000.

class PathFinder():

    ## grid is a list of 8 sub-lists, each containing 8 nodes.
    ## start and goal are tuples representing the grid indexes of my start and goal.

    def __init__(self, start, goal, grid):
        ## I don't do anything with these yet,
        ## just thought they would be good to have.
        self.goal = goal
        self.start = start
        self.grid = grid

    def find_path(self, start, goal, path=[]):

        ## My seemingly hackish method of looping through
        ## the 8 cells surrounding the one I'm on.
        for row in (-1,0,1):
            for col in (-1,0,1):
                if row == 0 and col == 0:
                    continue
                cell = [start[0]+row,start[1]+col]

                ## Catching the inevitable exception when I try to examine
                ## the surroundings of cells that are already on the edge.
                try:
                   ## Skip if the cell is blocked ("X").
                    if self.grid[cell[0]][cell[1]] == "X":
                        continue
                except IndexError:
                    continue

                path.append(cell)
                if cell == goal:
                    return path
                self.find_path(cell, goal, path)

def main():
    import Gridder2
    grid = Gridder2.board()
    start = (0,0)
    goal = (0,3)
    pt = PathFinder(start, goal, grid)
    print(pt.find_path(start, goal))

    return 0

if __name__ == '__main__':
    main()

My find_path function particularly seems like it needs work.  Seems like I should be able to calculate a path through an 8x8 grid with a recursion depth of less than 1000.

How can I improve this?

class PathFinder():

Either but object in the () or drop them. Having them there empty makes me feel hollow inside.

    ## grid is a list of 8 sub-lists, each containing 8 nodes.
    ## start and goal are tuples representing the grid indexes of my start and goal.

    def __init__(self, start, goal, grid):
        ## I don't do anything with these yet,
        ## just thought they would be good to have.
        self.goal = goal
        self.start = start
        self.grid = grid

    def find_path(self, start, goal, path=[]):

Don't put mutable values like list as your default parameters. Python will not create a new list everytime this function is called, rather it will keep reusing the same list which is probably not what you want

        ## My seemingly hackish method of looping through
        ## the 8 cells surrounding the one I'm on.
        for row in (-1,0,1):
            for col in (-1,0,1):
                if row == 0 and col == 0:
                    continue

I suggest creating a global constant with the 8 directions and doing: for row, col in DIRECTIONS:

                cell = [start[0]+row,start[1]+col]

Don't use lists unless they are actually lists. This should be a tuple.

                ## Catching the inevitable exception when I try to examine
                ## the surroundings of cells that are already on the edge.
                try:
                   ## Skip if the cell is blocked ("X").
                    if self.grid[cell[0]][cell[1]] == "X":
                        continue
                except IndexError:
                    continue

I suggest a function get_blocked that you pass cell to which check for 'X' and also returns True if the cell is off the side of the map. Also catching IndexError may not be quite what you want because negative numbers are valid indexes, but probably not want you want. I suggest checking in the get_blocked function for inboundedness.

                path.append(cell)
                if cell == goal:
                    return path
                self.find_path(cell, goal, path)

def main():
    import Gridder2

Usually not a good idea to import anywhere but at the start of a file

    grid = Gridder2.board()
    start = (0,0)
    goal = (0,3)
    pt = PathFinder(start, goal, grid)

pt?

    print(pt.find_path(start, goal))

Decide whether the start/goal get passed to the constructor or the find method. Don't do both.

    return 0

if __name__ == '__main__':
    main()

If you are going to return a value from your main function, you should pass it to sys.exit() so that it actually functions as a return code.

And everything Thomas said.


Python Cartesian Product in a constrained dictonary I want to calculate the Cartesian product of n copies of a small list, marker=[0,1,2]. I want to use these Cartesian product tuples as keys in a dictionary. The value per each key is to be a numpy array with n random floats between 0 and 1.

The only twist is that for each key:value pair in the dictionary, if the key has a non-zero number in its index a, I want the corresponding value np.array to have np.nan for the same index.

Below is the function I wrote for that. My question is whether there is a quicker / more efficient way to get the same result.

import itertools
import numpy as np
def create_constrained_dict(n, markers):
    '''
    Create cartesian product of a the same list repeated n times
    It returns a dictionary whose keys are the cartesian products of the
    input lists. The values of the dictionary are numpy arrays of length 'n'.
    If the corresponding dictionary key element for a value is not zero, we replace the value
    with np.nan.
    Belwo is an example:
    So for some key-value pair, NaN's would be lcoated as follows:
         d={(0,0,1): np.array([0.1234, 0.7543, np.nan]),
            (1,2,1): np.array([np.nan, np.nan, np.nan]),
            (1,0,1): np.array([np.nan, 0.2634, np.nan]),
    }

    '''
    d = dict()
    for element in itertools.product(*[markers  for i in xrange(n)]):
        d[element] = np.random.uniform(0, 1,n)
        for i in xrange(n):
            if element[i] !=0:
                d[element][i]= np.nan
    return d

rep_num = 3
marker = [0,1,2]
d = create_constrained_dict(rep_num, marker)

The output looks like this:

print d
{
  (0, 1, 1): array([ 0.84049621,         nan,         nan]),
  (0, 1, 2): array([ 0.17520962,         nan,         nan]),
  (1, 0, 1): array([        nan,  0.96110224,         nan]),
  (0, 2, 1): array([ 0.10395044,         nan,         nan]),
  (2, 2, 0): array([        nan,         nan,  0.60131589]),
  (0, 2, 0): array([ 0.64515576,         nan,  0.05946614]),
  (0, 2, 2): array([ 0.02054272,         nan,         nan]),
  (1, 0, 0): array([        nan,  0.98472074,  0.93688277]),
  (2, 0, 1): array([        nan,  0.64348266,         nan]),
  (1, 2, 0): array([        nan,         nan,  0.71462777]),
  (2, 0, 0): array([        nan,  0.98370414,  0.3517195 ]),
  (1, 2, 1): array([ nan,  nan,  nan]),
  (0, 0, 2): array([ 0.29771489,  0.83521032,         nan]),
  (2, 2, 2): array([ nan,  nan,  nan]),
  (1, 2, 2): array([ nan,  nan,  nan]),
  (2, 0, 2): array([        nan,  0.95682699,         nan]),
  (0, 0, 1): array([ 0.26649784,  0.38120757,         nan]),
  (0, 0, 0): array([ 0.98960411,  0.70080955,  0.25540202]),
  (2, 1, 2): array([ nan,  nan,  nan]),
  (1, 1, 1): array([ nan,  nan,  nan]),
  (0, 1, 0): array([ 0.94015447,         nan,  0.56849242]),
  (1, 1, 0): array([        nan,         nan,  0.30593067]),
  (2, 1, 0): array([        nan,         nan,  0.74205853]),
  (2, 2, 1): array([ nan,  nan,  nan]),
  (2, 1, 1): array([ nan,  nan,  nan]),
  (1, 1, 2): array([ nan,  nan,  nan]),
  (1, 0, 2): array([        nan,  0.27788722,         nan])
}

Instead of itertools.product(*[markers  for i in xrange(n)]) use itertools.product(markers, repeat=n)
Instead of creating three random values and replace it with nan use List Comprehensions.
dict([(key, value) for key, value in ...]) creates dict object.
[bool and [a] or [b]][0] - safer version of bool and a or b - one-linear version of:

if bool:
    a
else:
    b

And final version:

import itertools
import numpy as np

def create_constrained_dict(n, markers):
    d = dict([(element, np.array([(i == 0 and [np.random.uniform(0, 1)] or [np.nan])[0]
                                  for i in element]))
              for element in itertools.product(markers, repeat=n)])
    return d

EDIT

Version without np.array - 2 times faster (thanks @JoeWallis):

import itertools
import numpy as np

def create_constrained_dict(n, markers):
    d = dict([(element, [(i == 0 and [np.random.uniform(0, 1)] or [np.nan])[0]
                         for i in element]))
              for element in itertools.product(markers, repeat=n)])
    return d


Simple bash argument parser In a bash script, I need to parse arguments that have the following form:

The main arguments can be thought of as being a single argument, but I do not want to force users to quote the entire thing, so when the argument contains spaces I must be able to handle multiple arguments.
One flag may be passed as -<flag> where <flag> can be an arbitrary word (without spaces)
Finally, an external command, including its own options and flags may be passed. If so, this should be separated by a double dash.

For example,

my_command test

should result in

"$inp" == "test"
"$flag" == ""
"$ext_command" == ""

and

my_command this is a test -new -- sed "s|a|b|"

should result in

"$inp" == "this is a test"
"$flag" == "new"
"$ext_command" == "sed \"s|a|b\""

I think the following script does what I want, but since it's my first bash script, I wanted to ask whether the script is idiomatic and whether I missed any border cases.

local inp=""
local flag=""
local ext_command=""
local count="1"
local started=""
for i
do
    count=$((count+1))
    if [[ "$i" == '--' ]]
    then
        ext_command="${@:count}"
        break
    else
        if [[ "$i" == -* ]];
        then
           flag=${i#*-}
        else
            if [ ! "$started" ]
            then
                inp="$i"
                started=1
            else
                inp="$inp $i"
            fi
        fi
    fi
done

Don't go against the language

  The main arguments can be thought of as being a single argument, but I do not want to force users to quote the entire thing, so when the argument contains spaces I must be able to handle multiple arguments.

If you want to a value containing spaces as a single argument,
then you and your users should double-quote it,
otherwise Bash will perform word splitting.
This is a fundamental principle in Bash,
and it's better to play along with it than to go against.

Trying to go against will get you into all kinds of trouble.
For example, what would you expect for?

my_command this      is   a test -new -- sed "s|a|b|"

That is, multiple spaces between words.
Those spaces will be lost,
the script will behave the same way as if there was a single space in between.

Keep in mind that users will have to quote special characters anyway.
You cannot shelter them from quoting.
It's better to learn the basic rules of word splitting and quoting early,
rather than trying to work around it with hacky solutions.

Assign arrays to arrays

This statement assigns an array to a non-array:

ext_command="${@:count}"

This way you lose the ability to expand the original value correctly quoted.

Take for example this input:

my_command test -new -- sed "s|a| |"

Notice the space in the sed pattern.

And let's say the script uses ext_command like this:

ls | "$ext_command"

This will not work as intended (replacing "a" with spaces), because the original arguments are not preserved correctly.

Using an array you could leave this option open, that is:

ext_command=("${@:count}")

And then later:

ls | "${ext_command[@]}"

Minor points

Instead of this:

local inp=""

You can write simply:

local inp

Instead of this:

count=$((count+1))

You can write simply:

((count++))

Instead of this:

if [[ "$i" == '--' ]]
then
    ext_command="${@:count}"
    break
else
    # a long block of code ...
fi

It's more readable like this:

if [[ "$i" == '--' ]]
then
    ext_command="${@:count}"
    break
fi

# a long block of code ... but less deeply nested


Extension methods to make ConcurrentDictionary GetOrAdd and AddOrUpdate thread safe when using valueFactory delegates The ConcurrentDictionary<T,V> in .NET 4.0 is thread safe but not all methods are atomic.

This points out that:

  ... not all methods are atomic, specifically GetOrAdd and AddOrUpdate. The user delegate that is passed to these methods is invoked outside of the dictionary's internal lock.

Example Problem:

It is possible for the delegate method to be executed more than once for a given key.

public static readonly ConcurrentDictionary<int, string> store =
    new ConcurrentDictionary<int, string>();

[TestMethod]
public void UnsafeConcurrentDictionaryTest()
{

    Thread t1 = new Thread(() =>
    {
        store.GetOrAdd(0, i =>
        {
            string msg = "Hello from t1";
            Trace.WriteLine(msg);
            Thread.SpinWait(10000);
            return msg;
        });
    });

    Thread t2 = new Thread(() =>
    {
        store.GetOrAdd(0, i =>
        {
            string msg = "Hello from t2";
            Trace.WriteLine(msg);
            Thread.SpinWait(10000);
            return msg;
        });
    });

    t1.Start();
    t2.Start();
    t1.Join();
    t2.Join();
}

The result shown in the Trace window shows "Hello from t1" and "Hello from t2". This is NOT the desired behavior for most implementations that we are using and confirms the problem noted in the MSDN link above. What we want is for only one of those delegates to be executed.

Proposed Solution:

I have to use the delegate overloads of these methods which led me to investigate this matter further. I stumbled onto this post which suggests using the Lazy<T> class to ensure the delegate is only invoked once. With that in mind I wrote the following extension methods to mask the adding of a Lazy<T> wrapper to the value.

public static V GetOrAdd<T, U, V>(this ConcurrentDictionary<T, U> dictionary, T key, Func<T, V> valueFactory)
where U : Lazy<V>
{
    U lazy = dictionary.GetOrAdd(key, (U)new Lazy<V>(() => valueFactory(key)));
    return lazy.Value;
}

public static V AddOrUpdate<T, U, V>(this ConcurrentDictionary<T, U> dictionary, T key, Func<T, V> addValueFactory, Func<T, V, V> updateValueFactory)
where U : Lazy<V>
{
    U lazy = dictionary.AddOrUpdate(key,
                (U)new Lazy<V>(() => addValueFactory(key)),
                (k, oldValue) => (U)new Lazy<V>(() => updateValueFactory(k, oldValue.Value)));
    return lazy.Value;
}

Testing Solution:

Executing the same test above using a ConcurrentDictionary that has a Lazy value results in the value delegate ONLY being executed once (you either see "Hello from t1" or "Hello from t2")!

public static readonly ConcurrentDictionary<int, Lazy<string>> safeStore =
            new ConcurrentDictionary<int, Lazy<string>>();

So it seems that this approach accomplished the goal.

What do you think of this approach?

Allowing the caller to provide the type argument U implies that they are allowed to use a subclass of Lazy<V>, but this will not work as your implementations always creates a new List<V> and cast it to U. Since this means U must always be a Lazy<V> then why not do away with the extra type argument.

public static V GetOrAdd<T, V>(this ConcurrentDictionary<T, Lazy<V>> dictionary, T key, Func<T, V> valueFactory)

The name of the new extension methods conflict with the names of the existing methods. For the consumer to use yours instead of the existing methods, they would need to either access via your static class or use explicit type arguments. This could lead to subtle bugs when consumers try to use it as a extension method with type inference.

ExtensionHost.GetOrAdd(safeStore, 7, (i) => i.ToString());             // uses yours
safeStore.GetOrAdd<int, Lazy<string>, string>(6, (i) => i.ToString()); // uses yours
safeStore.GetOrAdd(5, (i) => i.ToString());                            // uses existing


What's more idiomatic in Haskell? I'm writing the data structures for my program (a toy compiler), and I'm trying to understand what's the best way to define the AST:

Current code

data RExpr = RExpr Location InnerRExpr
data InnerRExpr = RLExpr LExpr | RConstant Constant | RMathExpr MathExpr | FCall Id [RExpr]

Alternative A

data RExpr = RLExpr Location LExpr
           | RConstant Location Constant
           | RMathExpr Location MathExpr
           | FCall Location Id [RExpr]

Alternative B

data RExpr = RLExpr { loc::Location, getLexpr::LExpr}
           | RConstant { loc::Location, getConstant::Constant}
           | RMathExpr { loc::Location, getExpr::MathExpr}
           | FCall { loc::Location, id::Id, params::[RExpr] }

Honestly I'm not satisfied with either of the three options, because the current code means that I have an extraneous object in the AST which doesn't really mean anything, alternative A means that I have to pattern match every time I want to extract the location (or write a boilerplate function that does it) and alternative B means cluttering the global namespace with functions whose names are likely to collide.

Suggestions?

I tend towards the former, and then use a lens or traversal to extract the location.

import Control.Lens

class HasLocation t where
  loc :: Lens' t Location

instance HasLocation RExpr where
  loc f (RLExpr l e)    = f l <&> \l' -> RLExpr l' e
  loc f (RConstant l c) = f l <&> \l' -> RConstant l' c
  loc f (RMathExpr l m) = f l <&> \l' -> RMathExpr l' m
  loc f (FCall l i xs) = f l <&> \l' -> FCall l' i xs

With the class you can overload the use of loc for other data types. Because it is a lens you can use it to get/set/modify the location with a large vocabulary without cluttering your namespace.


localStorage setExpire code to add time expires Storage.prototype.setExpire = function (arrObj) {
    var date,
        now,
        days,
        deletes,
        items,
        fa, ta,
        newItems = [],
        storage = localStorage.getItem('limitStorage');
    items = JSON.parse(storage || "[]");
    date = new Date();
    date = date.toString().split(' ');
    date = Number(date[2]);
    for (var key in arrObj) {
        for (i = 0; i < items.length; i++) {
            if (items[i].value == key) {
                if (typeof (items[i].days) == "number") {
                    if (items[i].date !== date) {
                        days = (date - items[i].date);
                    } else {
                        days = items[i].days;
                    }
                }
            }
        }
        ta = {
            value: key,
            limit: arrObj[key],
            date: date,
            days: days !== undefined ? days : 0
        };
        if (ta.days >= ta.limit) {
            localStorage.removeItem(ta.value);
        }
        if(localStorage.getItem(ta.value) !== null) newItems.push(ta);
    }
    localStorage.setItem('limitStorage', JSON.stringify(newItems));
};

This code runs by simply calling the Storage method and then adding method .setExpire.

Example:

window.localStorage.setExpire({"item1":3,"item2":10,"item3":3});

I have to add more to the code to take into account day 31 to day 1.  But as of now, if the item is added on day 20 and you want it to expire in 7 days on day 27, the localStorage item will be removed.

My question is: does anyone see anything they can review for me? Maybe a cross-browser compatibility issue?  I already have a cross-browser localStorage that falls back to cookies if need be.

Also let me know if you may think of a way for the code to take into account the ending to the start of a months, since the code technically only takes the new date and then subtracts the stored date, which gets our days that have passed.

Any and all suggestions are welcomed.

I think this code could use a lot of polishing.

Getting the localStorage

 storage =localStorage.getItem('limitStorage');
 getItems = storage !== null && storage !== "[]" ? localStorage.getItem('limitStorage') : null;
 getItems = getItems !== null ? JSON.parse(getItems) : null;

can easily be replaced with:

items = JSON.parse( localStorage.getItem('limitStorage') || "[]" );

I renamed getItems to items ( getItems is a name better suited for a function ), the code also defaults items to an empty array if nothing was found.

Setting an expiry date

The easiest way to deal with that is to get now in milliseconds, add 1000 * 60 * 60 * 24 * days to it, and set that as the expiry date. Then you only have to compare (new Date()).getTime() with the expiry date.

So you have something like

var now = (new Date()).getTime();
var day = 1000 * 60 * 60 * 24;

for (var key in arrObj) {
  newItems.push( { value : key , expires : now + day * arrObj[key] } );

Now, we are facing 2 problems:

If you want to change the expiry date of an item, you cant..
Much worse, once you set some items to expire, you can never add extra items to expire since you will always fall into the else branch.

At this point, I think the whole code needs a re-write and re-think.


Replacing a variable in a loop by records from a Table I have a Date Table like this one:

Exit_Date  Date_ID
2017-05-31    1
2017-04-26    2
2017-01-02    3
2016-12-24    4
2016-11-27    5

I use a loop to insert each of those Dates into a CTE to generates the last 15 years of these dates like this:

declare @DI int = 1
declare @d date
while @DI <=5
begin
    select @d = Exit_Date from Date_Table where Date_ID = @DI
    declare @EDTable table (Exit_Date  Date);
    with
      a as(
        select dateadd(yy,-1,@d) d,0 i
          union all
        select dateadd(yy,-1,d),i+1 from a where i<14
      ),
      b as(select d,datediff(dd,0,d)%7 dd from a)
     insert into @EDTable select d from b;
      set @DI = @DI + 1
 end

The results is correct, I get 75 rows with my dates. I would like to know if there is a way to get rid of the WHILE loop by replacing variable @d by each date record from Date_Table?

Selecting from Date_Table in a
One option is to select Exit_Date direct from Date_Table in a. The ordering of the end results won't be the same so you may need to add an ORDER BY clause (e.g. with DATEPART()).
 with
      a as(
        select dateadd(yy,-1,Exit_date) d,0 i
        from Date_Table WHERE Date_ID <=5
          union all
        select dateadd(yy,-1,d),i+1 from a where i<14
      ),
      b as(select d,datediff(dd,0,d)%7 dd from a)
SELECT d from b --order by DATEPART(MM,d) desc

See a sample here in this SQLFidle.
Cursor
Like I mentioned in my comment, another option is to use a Transact-SQL cursor. In the squery below, we keep the temp variables @d and @EDTable. The SELECT statement from Date_Table could have ORDER BY Date_ID ASC added if you wanted to ensure those go in order (in case the records were not added sequentially).
declare @d date;
declare @EDTable table (Exit_Date  Date);
declare dateCursor CURSOR FOR
    select Exit_Date from Date_Table WHERE Date_Id <=5;

OPEN dateCursor

FETCH NEXT FROM dateCursor INTO @d

WHILE @@FETCH_STATUS = 0
BEGIN
    with
      a as(
        select dateadd(yy,-1,@d) d,0 i
          union all
        select dateadd(yy,-1,d),i+1 from a where i<14
      ),
      b as(select d,datediff(dd,0,d)%7 dd from a)
     insert into @EDTable select d from b;--*/
    FETCH NEXT FROM dateCursor
    INTO @d
END
CLOSE dateCursor;
DEALLOCATE dateCursor;

SELECT * FROM @EDTable

I am trying to get a SQL Fiddle working but having issues with the line endings. I will update when I figure that out.


Get collection in blade and check if empty If I want to display the addresses of a user in a blade file I do it like this:

@if(!$user->addresses->isEmpty())

  @foreach ($user->addresses as $address)

      {!! displayText($address->toString()) !!}

  @endforeach

@endif

If addresses would be an array instead of an collection I could shorten the code to:

@if(!empty($addresses = $user->addresses))

  @foreach ($addresses as $address)

      {!! displayText($address->toString()) !!}

  @endforeach

@endif

I wonder if there is a better way to check if a collection is empty in a blade file.

You don't need to check if your array is empty before using it in a foreach.

But if you need to check if it is empty (to show a message, for example), you can use the @empty tag.

I'd do something like this: (Check also @forelse):

  @forelse ($addresses as $address)

      {!! displayText($address->toString()) !!}

  @empty($addresses)

 No addresses...

  @endforelse


C function delimiates strings using a single character array I am a complete noob to C and I've shied away from using such a powerful language due to it's large learning curve and other complicated mechanisms. For my first minor project I plan to create a small HTTP client that requests for a webpage. Before I start, I need to create a function that can correctly parse a URL using the URI scheme. I devised this function to help me separate http from a host name like www.google.com by recognizing ://. Space and clock efficiency is are priorities of mine. I'm looking for any tips that would help someone who is new to C.

int delimiateString(const char *string, const char *delimiator) {
    const char *buffer;
    int x, len = strlen(delimiator);
    for (buffer = string; *buffer != '\0'; buffer++) {
        if (*delimiator == *buffer) {
            for (x = 1; x <= len; x++) {
                if (*(delimiator+x) == *(buffer+x)) {
                    if (x+1 == len) {
                        return (buffer - string);
                    }
                } else {
                    break;
                }
            }
        }
    }
    return strlen(string);
}

Your function seems to do this:

Find the first occurrence of "delimiator" (delimiter?) in some string. For example find the first occurrence of :// in http://example.com.
If found, return the position where it was found
If not found, return the length of the input string

To find if a string is part of another string, you can use the strstr function.
This is equivalent to your code but simpler:

int delimiateString(const char *string, const char *delimiator) {
    char * found = strstr(string, delimiator);
    return found != NULL ? found - string : strlen(string);
}

For the record,
your original code can be simplified,
by eliminating an if around the inner for loop,
and rearranging the nested ifs at the deepest level:

for (buffer = string; *buffer != '\0'; buffer++) {
    for (x = 0; x < len; x++) {
        if (*(delimiator+x) != *(buffer+x)) {
            break;
        }
        if (x+1 == len) {
            return buffer - string;
        }
    }
}


Counting characters, digits, spaces and newlines in the loaded file I wrote an application that loads a file after pressing button and counts characters, digits, spaces and newlines in the loaded file. This application is written in Python language using tkinter library and OOP programming paradigm.

4programmers.py:

# -*- coding: utf-8 -*-
from Tkinter import *
import Tkinter, Tkconstants, tkFileDialog

class GUI:
    def __init__(self, root):
        self.root = root
        self.filePath = StringVar()
        self.filePath.set("File not loaded")
        self.charLength = StringVar()
        self.charLength.set(0)
        self.digitsLength = StringVar()
        self.digitsLength.set(0)
        self.spacesLength = StringVar()
        self.spacesLength.set(0)
        self.entersLength = StringVar()
        self.entersLength.set(0)

        root.title("A program that counts characters, digits, spaces and newlines in the given file.")

        topFrame1 = Frame(root)
        topFrame1.pack( side = TOP )
        Label(topFrame1, text="Chars count: ").pack( side = LEFT )
        lblCharLength = Label(topFrame1, textvariable=self.charLength)
        lblCharLength.pack( side = RIGHT )

        topFrame2 = Frame(root)
        topFrame2.pack( side = TOP )
        Label(topFrame2, text="Digits count: ").pack( side = LEFT )
        lblDigitsLength = Label(topFrame2, textvariable=self.digitsLength)
        lblDigitsLength.pack( side = RIGHT )

        topFrame3 = Frame(root)
        topFrame3.pack( side = TOP )
        Label(topFrame3, text="Spaces count: ").pack( side = LEFT )
        lblSpacesLength = Label(topFrame3, textvariable=self.spacesLength)
        lblSpacesLength.pack( side = RIGHT )

        topFrame4 = Frame(root)
        topFrame4.pack( side = TOP )
        Label(topFrame4, text="Newlines count: ").pack( side = LEFT )
        lblEntersLength = Label(topFrame4, textvariable=self.entersLength)
        lblEntersLength.pack( side = RIGHT )

        bottomFrame = Frame(root)
        bottomFrame.pack( side = BOTTOM )

        lblFileName = Label(bottomFrame, textvariable=self.filePath)
        lblFileName.pack( side = LEFT )

        btnReadFile = Button(
                bottomFrame,
                text="Load a file",
                fg="black",
                command=self.importFile)
        btnReadFile.pack( side = RIGHT )

    def importFile(self):
        self.filePath.set(tkFileDialog.askopenfilename())

        s = open(self.filePath.get()).read()
        self.charLength.set(len(s))
        self.digitsLength.set(sum(c.isdigit() for c in s))
        self.spacesLength.set(sum(c.isspace() for c in s))
        self.entersLength.set(sum(c == "\n" for c in s))
        self.root.update()

root = Tk()
gui = GUI(root)
root.mainloop()

Don't use wildcard imports

PEP8 says that wildcard imports should be avoided. Instead, I recommend you import tkinter like this:

import Tkinter as tk

This means that everywhere you use a tkinter class or constant you need to prefix it with tk.. This will make your program a bit easier to read and understand, especially if you eventually choose to import widgets from ttk which provides many classes with the same name as tkinter classes.

Group layout commands together

You have calls to pack scattered throughout your code, making it difficult to visualize how things are laid out. I strongly recommend grouping your layout code based on parent (ie: call pack on all children that share the same parent)

For example:

topFrame1 = Frame(root)
lbl = Label(topFrame1, text="Chars count: ")
lblCharLength = Label(topFrame1, textvariable=self.charLength)

# pack all children of topFrame1 together
lbl.pack( side = LEFT )
lblCharLength.pack( side = RIGHT )

lbl = Label(topFrame2, text="Digits count: ")
lblDigitsLength = Label(topFrame2, textvariable=self.digitsLength)

# pack all children of topFrame2 together
lbl.pack( side = LEFT )
lblDigitsLength.pack( side = RIGHT )

...

# pack all of the children of root together
topFrame1.pack( side = TOP )
topFrame2.pack( side = TOP )
topFrame3.pack( side = TOP )
topFrame4.pack( side = TOP )
bottomFrame.pack( side = BOTTOM )

You don't need StringVar

You don't need to use StringVar in your code. The way you are using it means you have more objects, but these objects don't give you any added benefits. Thus, it makes your code more complex than it needs to be.

If you need to change a label at runtime, you can keep a reference and then use the configure method to directly change the label.

self.lblCharLength = Label(topFrame1, textvariable=self.charLength)
...
self.lblCharLength.configure(text=len(s))

If you choose to use them, I recommend setting the value at the time  you create them, which will cut down on the total lines of code:

self.filePath = StringVar("File not loaded")
self.charLength = StringVar(value=0)
self.digitsLength = StringVar(value=0)
self.spacesLength = StringVar(value=0)
self.entersLength = StringVar(value=0)

Use with to open and read a file

with makes it easy to write code that is wrapped by other code. In this case you want to do an open, read some code, and then do a close.

Note: you should also verify that the user picked a path. As written, if the user presses cancel, self.filePath will be empty and your code will throw an error.

Also, when you try to cram two or more function calls into a single statement it can make the code harder to debug. By using intermediate variables it makes it easier to pause your program and examine the state.

For example:

if filepath:
    self.filePath.set(filepath)   # if using StringVar
    # self.filePath.configure(text=filepath)   # if not using StringVar

    with open(filepath, "r") as f:
        s = f.read()
    ...

Avoid calling update

At the end of your function importFile you call self.root.update. That is completely superfluous. The screen will update as soon as the function returns.