Checking multiple XML in PHP I am working on a code to see if a video already exists. The problem is that I have to check multiple sections. The code is working but it should not be this big, I think, and it takes a long time to load.

Is there an easier way to do this?

$xmlurl7b = 'http://mysite.me/library/sections/2/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl1b = 'http://mysite.me/library/sections/5/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl2b = 'http://mysite.me/library/sections/7/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl3b = 'http://mysite.me/library/sections/9/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl4b = 'http://mysite.me/library/sections/10/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl5b = 'http://mysite.me/library/sections/11/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl6b = 'http://mysite.me/library/sections/13/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl8 = 'http://mysite.me/library/sections/3/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl9 = 'http://mysite.me/library/sections/8/all?title=' . $movie->title() . '&HndJk7Sd';
$xmlurl7 = 'http://mysite.me/library/sections/2/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl1 = 'http://mysite.me/library/sections/5/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl2 = 'http://mysite.me/library/sections/7/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl3 = 'http://mysite.me/library/sections/9/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl4 = 'http://mysite.me/library/sections/10/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl5 = 'http://mysite.me/library/sections/11/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$xmlurl6 = 'http://mysite.me/library/sections/13/all?guid=movieid' . $movieid . '%3Flang%3Dnl&HndJk7Sd';
$achxml8 = simplexml_load_file($xmlurl8);
$achxml9 = simplexml_load_file($xmlurl9);
$achxml1 = simplexml_load_file($xmlurl1);
$achxml2 = simplexml_load_file($xmlurl2);
$achxml3 = simplexml_load_file($xmlurl3);
$achxml4 = simplexml_load_file($xmlurl4);
$achxml5 = simplexml_load_file($xmlurl5);
$achxml6 = simplexml_load_file($xmlurl6);
$achxml7 = simplexml_load_file($xmlurl7);
$achxml1b = simplexml_load_file($xmlurl1b);
$achxml2b = simplexml_load_file($xmlurl2b);
$achxml3b = simplexml_load_file($xmlurl3b);
$achxml4b = simplexml_load_file($xmlurl4b);
$achxml5b = simplexml_load_file($xmlurl5b);
$achxml6b = simplexml_load_file($xmlurl6b);
$achxml7b = simplexml_load_file($xmlurl7b);
if( $achxml1->Video && ($achxml1->Video != '')
OR $achxml2->Video && ($achxml2->Video != '')
OR $achxml3->Video && ($achxml3->Video != '')
OR $achxml4->Video && ($achxml4->Video != '')
OR $achxml5->Video && ($achxml5->Video != '')
OR $achxml6->Video && ($achxml6->Video != '')
OR $achxml7->Video && ($achxml7->Video != '')
OR $achxml8->Directory && ($achxml8->Directory != '')
OR $achxml9->Directory && ($achxml9->Directory != '')
OR $achxml1b->Video && ($achxml1b->Video != '')
OR $achxml2b->Video && ($achxml2b->Video != '')
OR $achxml3b->Video && ($achxml3b->Video != '')
OR $achxml4b->Video && ($achxml4b->Video != '')
OR $achxml5b->Video && ($achxml5b->Video != '')
OR $achxml6b->Video && ($achxml6b->Video != '')
OR $achxml7b->Video && ($achxml7b->Video != '')
 ) {
echo 'Exists';
} else {
echo 'New';
}

This is a giant mess! Here's what you should do to start cleaning it up.

Code organization

Everything you are doing is quite repetitive with minimal variations. Start by constructing an array of each section number you need to inspect for title and each section number you need to inspect by guid. You can then iterate through these systematically without writing everything out.
Turn these two strings into constants you define at the top of your file '%3Flang%3Dnl&HndJk7Sd'  and '&HndJk7Sd'. SImilarly does $movie->title() return the same value each time you call it? If so, move this value into a constant too.
I would then do a loop of a kind like this (pseudocode):

for each filename in possible files{
 video = simplexml_load_file(filename)
 if(video->Video && video->Video!='') {
     echo 'Exists'
     gracefully exit script?
}
//If script has not exited file doesn't exist
echo 'New';

Note that this general organization means you stop as soon as you find a file. The way you have set up your code now, you load all directories but you might have actually found the video in the very first place you checked.

PHP language features

Any reason the standard function file_exists() isn't applicable in some way? Not sure why you seem to have all these special functions just to check if a file exists?


shared_ptr and FILE for wrapping cstdio (update: also dlfcn.h) Even in the presence of <fstream>, there may be reason for using the <cstdio> file interface. I was wondering if wrapping a FILE* into a shared_ptr would be a useful construction, or if it has any dangerous pitfalls:

#include <cstdio>
#include <memory>

std::shared_ptr<std::FILE> make_file(const char * filename, const char * flags)
{
  std::FILE * const fp = std::fopen(filename, flags);
  return fp ? std::shared_ptr<std::FILE>(fp, std::fclose) : std::shared_ptr<std::FILE>();
}

int main()
{
  auto fp = make_file("hello.txt", "wb");
  fprintf(fp.get(), "Hello world.");
}

Update: I just realized that it is not allowed to fclose a null pointer. I modified make_file accordingly so that in the event of failure there won't be a special deleter.

Second update: I also realized that a unique_ptr might be a more suitable than shared_ptr. Here is a more general approach:

typedef std::unique_ptr<std::FILE, int (*)(std::FILE *)> unique_file_ptr;
typedef std::shared_ptr<std::FILE> shared_file_ptr;

static shared_file_ptr make_shared_file(const char * filename, const char * flags)
{
  std::FILE * const fp = std::fopen(filename, flags);
  return fp ? shared_file_ptr(fp, std::fclose) : shared_file_ptr();
}

static unique_file_ptr make_file(const char * filename, const char * flags)
{
  return unique_file_ptr(std::fopen(filename, flags), std::fclose);
}

Edit. Unlike shared_ptr, unique_ptr only invokes the deleter if the pointer is non-zero, so we can simplify the implementation of make_file.

Third Update: It is possible to construct a shared pointer from a unique pointer:

unique_file_ptr up = make_file("thefile.txt", "r");
shared_file_ptr fp(up ? std::move(up) : nullptr);  // don't forget to check

Fourth Update: A similar construction can be used for dlopen()/dlclose():

#include <dlfcn.h>
#include <memory>

typedef std::unique_ptr<void,  int (*)(void *)> unique_library_ptr;

static unique_library_ptr make_library(const char * filename, int flags)
{
  return unique_library_ptr(dlopen(filename, flags), dlclose);
}

I should start with the fact that I don't entirely agree with the widespread belief that "explicit is better than implicit". I think in this case, it's probably at least as good to have a class that just implicitly converts to the right type:

class file {
    typedef FILE *ptr;

    ptr wrapped_file;
public:
    file(std::string const &name, std::string const &mode = std::string("r")) :
        wrapped_file(fopen(name.c_str(), mode.c_str()))
    { }

    operator ptr() const { return wrapped_file; }

    ~file() { if (wrapped_file) fclose(wrapped_file); }
};

I haven't tried to make this movable, but the same general idea would apply if you did. This has (among other things) the advantage that you work with a file directly as a file, rather than having the ugly (and mostly pointless) .get() wart, so code would be something like:

file f("myfile.txt", "w");

if (!f) {
   fprintf(stderr, "Unable to open file\n");
   return 0;
}

fprintf(f, "Hello world");

This has a couple of advantages. The aforementioned cleanliness is a fairly important one. Another is the fact that the user now has a fairly normal object type, so if they want to use overloading roughly like they would with an ostream, that's pretty easy as well:

file &operator<<(file &f, my_type const &data) {
    return data.write(f);
}

// ...

file f("whatever", "w");
f << someObject;

In short, if the user wants to do C-style I/O, that works fine. If s/he prefers to do I/O more like iostreams use, a lot of that is pretty easy to support as well. Most of it is still just syntactic sugar though, so it generally won't impose any overhead compare to using a FILE * directly.


Selecting a thumbnail from among a large, medium, and small image I'm parsing some data in C# where I have an object with 3 properties: "large", "medium", "small"; each property may contain a string array with an image path, and I'm trying to extract that path.

The data should always have a large image, but not always a medium or small. I'm trying to generate thumbnail images, so I prefer the small, but if it's not there I want the medium, otherwise the large. The following code works and I understand it but I have the feeling there's a more expressive way:

var thumb = (images == null)
            ? null
            : (images.small == null) || (images.small.Length == 0)
                ? (images.medium == null) || (images.medium.Length == 0)
                    ? (images.large == null) || (images.large.Length == 0)
                        ? null
                        : images.large[0]
                    : images.medium[0]
                : images.small[0];

I could at least break out the guard clauses to a Boolean method like hasImage("small") or better yet hasImage(imageEnum.SMALL) -- that would make the tests easier to understand and also encapsulate them so changes to the test logic wouldn't pollute the retrieval logic.

Other than that, is there a better (i.e. easier to understand) way to express the overall algorithm?

var thumb = (images == null)
            ? null
            : (images.small == null) || (images.small.Length == 0)
                ? (images.medium == null) || (images.medium.Length == 0)
                    ? (images.large == null) || (images.large.Length == 0)
                        ? null
                        : images.large[0]
                    : images.medium[0]
                : images.small[0];

  I understand it but

no one else does ;-)

Ternary operators are very usefull but you really exaggerated. In a production code I would never accept it.

You should break it to smaller pieces.

  is there a better (i.e. easier to understand) way to express the overall algorithm?

There might be. If the images type is under your control then you could implement the IEnumerable interface on it and return the sub-image collections:

class ImageCollection : IEnumerable<Bitmap[]>
{
    public Bitmap[] small { get; set; }
    public Bitmap[] medium { get; set; }
    public Bitmap[] large { get; set; }

    public IEnumerator<Bitmap[]> GetEnumerator()
    {
        yield return small;
        yield return medium;
        yield return large;
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return this.GetEnumerator();
    }
}

Then with a small helper lambda you can encapsulate the first-image-or-default logic and use linq to retrieve the first image found:

var images = new ImageCollection();
var getFirstImageOrDefault = new Func<Bitmap[], Bitmap>(
    bitmaps => bitmaps == null ? null : bitmaps.FirstOrDefault());
var thumb = images.Select(getFirstImageOrDefault).FirstOrDefault(bitmap => bitmap != null);


Grouping orders by similarity (cluster) of their items My task is to write an algorithm for grouping list of orders into batches, each batch consisting of 4 orders. Orders are grouped by similarity of their items, which means the more items from Order X sharing the same location as Order Y's items, the bigger the similarity ratio between both. Note that one order is compared only against equally sized or bigger orders. A similarity ratio of 0 would mean no items match, whereas 1 means 100% match. Also, the biggest order in a batch has ratio of 1 because it's compared only against itself.

The idea behind all this is to optimize the path for a warehouse worker who visit a location and pick the ordered items. In the best case they should pick all identical items with a single visit, then proceed to the next location.

The code below represents a custom class I created for this purpose, but something says me it's not very optimized or could be written better. I'd be very happy if you can help me out and give some advice on what could be improved.

public static class SimilarityCalculator
{
    private static double CalcSimilarityRatio(Order minOrder, Order maxOrder)
    {
        var similarities = 0;

        foreach (var item1 in minOrder.Items)
        {
            foreach (var item2 in maxOrder.Items)
            {
                if (string.Equals(item1.Cluster, item2.Cluster))
                {
                    similarities++;
                    break;
                }
            }
        }

        var ratio = (double)similarities / minOrder.Items.Count;
        ratio = Math.Round(ratio, 3);
        return ratio;
    }

    private static void CheckSimilarities(List<Order> ordersList)
    {
        foreach (var order in ordersList)
        {
            order.MatchingList.Clear();

            var equalOrGreaterOrders = ordersList
                                       .Where(o => o.Items.Count >= order.Items.Count)
                                       .ToList();

            foreach (var equalOrGreater in equalOrGreaterOrders)
            {
                var ratio = CalcSimilarityRatio(order, equalOrGreater);
                var match = new KeyValuePair<Order, double>(equalOrGreater, ratio);
                order.MatchingList.Add(match);
            }
        }
    }

    private static List<KeyValuePair<Order, double>> GetTop4Matches(Order order)
    {
        var top = (from m in order.MatchingList
                   orderby m.Value descending
                   select m)
                   .Take(4)
                   .ToList();

        return top;
    }

    /// <summary>
    /// This method is called for creating the batches. Can be used from another app.
    /// </summary>
    /// <param name="ordersList">All the orders that should be grouped into batches.</param>
    /// <returns>Generated batches consisting of 4 orders each.</returns>
    public static List<List<Order>> GenerateBatches(List<Order> ordersList)
    {
        var batches = new List<List<Order>>();
        while (ordersList.Any())
        {
            CheckSimilarities(ordersList);

            var currentOrder = ordersList.First();
            var top4Matches = GetTop4Matches(currentOrder);
            var batch = new List<Order>();

            foreach (var match in top4Matches)
            {
                batch.Add(match.Key);
                ordersList.Remove(match.Key);
            }

            batches.Add(batch);
        }

        return batches;
    }
}

Faulty algorithm.  Any order would be 100% to an order of all items.

I real life a warehouse worker can only pick up so many items in a pass.

Take an order less than pass size and find the order or orders fully contained that get up to pass size.

After you cannot get up to pass size contained then normalize similarity with match / (size1 + size2).

A better measure of similarity is:

var ratio = 2 * (double)similarities / (minOrder.Items.Count + maxOrder.Items.Count);


Luhn checksum in Python I've implemented Luhn checksum in Python version 3. I want your comments and opinion about my code. I know it's embarrassing code and that it doesn't follow python best practices but it works!.

The x in doubledigit parameter means that if the boolean is False, the index of the number is odd so we don't have to double it, if it is True it means the index is even, so we double it.

import ast

def  doubledigit(n,x) :

    if x is  True:

        doubledigit = n *2
        sum=0
        if doubledigit >= 10 :
            sum = 1 + doubledigit % 10
        else: sum = doubledigit
    else:
        sum = n

    return (sum)
def sum ():
    total =0
    prop = int(input("enter how many digits you want: " ))
    for i in range (0,prop):
        n = int(input("Enter your digit: " ))
        x = input("Enter True or False: ")
        total = total+doubledigit(n,ast.literal_eval(x))

    return total
def findx (n):
    if (n % 10 == 1 ):
        print("the check digit is 9")

        n += 9
    if (n % 10 == 2 ):
        print("the check digit is 8")

        n += 8
    if (n % 10 == 3 ):
        print("the check digit is 7")
        n += 7
    if (n % 10 == 4 ):
        print("the check digit is 6")

        n += 6
    if (n % 10 == 5 ):
        print("the check digit is 5")

        n += 5
    if (n % 10 == 6 ):
        print("the check digit is 4")

        n += 4
    if (n % 10 == 7 ):
        print("the check digit is 3")

        n += 3
    if (n % 10 == 8 ):
        print("the check digit is 2")

        n += 2
    if (n % 10 == 9 ):
        print("the check digit is 1")

        n += 1
    if (n % 10 == 0):
        print("vaild checksum")
    return n
print(findx(sum()))

Coding style

First, have a read through the Python style guide, PEP 8.
There's a lot of inconsistency in your coding style, and with the style guide, you should be able to remove all that. It will then also become consistent in style with a lot of existing Python code.

With that in hand, you can fix the following:

often, single space instead of double space.
Not if x  is True, but if x is True.
surround operators with a space, except as keyword arguments.
Not total =0, but total = 0.
Not  sum=0, but sum = 0.
leave a whitespace after a comma.
Not def  doubledigit(n,x) :, but def doubledigit(n, x):
(note: that also removed a double space, and remove the space before the :.)
remove parentheses from return statements and in conditions.
Not: return (sum), but return sum.
Not: if (n % 10 == 1 ):, but if n % 10 == 1:
avoid putting things on the same line after a colon.
Not else: sum = doubledigit, but

else:
    sum = doubledigit
If an argument in a condition (eg, if-statement) has a truthy or falsy value, or is simply True or False, don't compare it to anything (exceptions exist, but not so much here), but use the value itself.
Not: if x is  True: but if x:
Remove unnecessary line breaks for code that belongs together, such as in your if blocks. Do insert an extra line break after a function ends.
Thus: an extra line break after return sum and return total.
But remove the line breaks after print("the check digit is 8") and similar lines, because the next line definitely belongs to that if-clause.
Avoid overwriting built-int variables and functions.
Not: def sum():, but e.g. def calc_total()
and for the variable named sum, use a different name like total.
(I've used calc_total() here to avoid clashes with the variable named total, although such double naming will often work. It is, however, also a good idea to use a verb in your function name, to show it does something (calc is a bit generic, perhaps).

Here's the list with built-in functions
Use clearly named variables. n is just about acceptable (assuming it's short for number, but x is too generic. I don't know the algorithm well enough to come up with an appropriate name, but something like double_flag could be a decent name (tad long perhaps).

The same holds for function names: findx is rather generic, and from the name, I have no idea what we're trying to find here (relevant joke).

User input

Have a look at your input statements. There is a direct conversion to integer, which will cause a crash if someone accidentally enters, say 'a'. A crash is better than using, e.g., the value 97, but ideally, some user feedback is nice. Consider:

while True:
    n = input("Enter your digit: ")
    try:
        n = int(n)
        break
    except ValueError:
        print("Please enter a number", file=sys.stderr)

and similar for prop.
This costs more effort (and code), but when dealing with user interaction, there'll be a lot of extra code (never think "this will never happen for this code", because it will).
(Add an import sys at the top of your code, for the output to standard error.)

Note: I've re-used n here, first as the result of input() (being a str), then promptly assigning it to its actual, integer value. Since these lines are close together, and within the same while block, I don't feel this is an issue, but usually, it's better to use different variable names. E.g., answer = input(...) and then n = int(answer). See also Konrad Rudolph's comment below.

For the true or false input, avoid the ast.literal_eval.  Just compare directly to "True" or "False" input strings, and set x accordingly:

while True:
    x = input("Enter True or False: ").lower()
    if x in ['true', 'false']:
        x = True if x == 'true' else False
        break
total = total + doubledigit(n, x)

(I've taken the liberty to allow "true", "falSe" etc as input as well.)

I've added a short, ternary if-statement: x = True if x == 'true' else False. If that's new and confusing, it's exactly the same as:

if x == 'true':
    x = True
else:
    x = False

Alternatively, since a comparison yields a True or False value, you can also simply use:

x = x == 'true'

as Konrad Rudolph in the comments points out.

If chain

Your set of if statements all look remarkably similar. And if you check the operations inside the if statement and following lines, you may notice that while one set of digits increase, the other set decreases.

In fact, checkdigit = 10 - n % 10, and with that, all your if statements can be removed, and replaced with just this:

checkdigit = 10 - n % 10
print("the check digit is", checkdigit)
n += checkdigit

Of course, there is the n % 10 == 0 case, where the message changes, and no addition takes place. So:

checkdigit = 10 - n % 10
if checkdigit == 10:
    print("valid checksum:")
else:
    print("the check digit is", checkdigit)
    n += checkdigit

Using Python code as script or module

Your call to findx is at the top level of your code (at the bottom). That is, if you ever import your script (say, import luhn), the code will immediately execute. That is fine as a script (run as python luhn.py), but rarely when importing.

To avoid, a slightly fancy trick is applied, that'll you find in lots of places: we can check whether a file is imported or executed as a script. In the latter case, the special variable __name__ takes the value "__main__" (a string). Testing for that in an if statement avoids running the code when imported:

if __name__ == "__main__":
    print(findx(sum()))

Now you can import luhn without a problem, and then manually run luhn.findx(sum()) at your leisure (or, for example, just luhn.sum()).

Nested call

You call findx inside a print statement, and you calculate sum inside the call to findx. For clarity, it is nicer to separate this (there are cases where this is fine, but in general, separating them makes it easier to follow the program flow):

total = sum()
x = findx(total)
print(x)

Comments and doc strings

Python has the great ability to allow documentation inside the code, with so-called docstrings. Ideally, every script, function etc. has a docstring. Sometimes short, sometimes long. It describes what the function does, what inputs it expects, what errors can be raised. I haven't really bothered here with it, but it could be good to have them.
It would look a bit like:

def doubledigit(n, x):
    """Calculate the sum for double digits

    Calculate the sum for double digits, with the following algorithm:
    <brief description of algorithm>

    Inputs:
    - n: an integer
    - x: True or False

    Returns:
    The sum of double digits

    """

Have a read through the conventions used, or search around for more about Python docstrings: they are immensely useful if you start writing larger scripts, modules, packages, since they automatically provide documentation for both yourself (in a months time) and other users.

Algorithm

The one thing I haven't done, is verifying the correctness of the actual algorithm. Creating some automated tests (unit tests) would be good, but that's a step advanced.

With all the above applied, and some other minor fixes (e.g., sum = 0 is unnecessary, since it gets assigned either way in the lines below), my version of your code now looks as follows:

import sys

def doubledigit(n, double_flag):
    if double_flag:
        doubledigit = 2 * n
        if doubledigit >= 10 :
            total = 1 + doubledigit % 10
        else:
            total = doubledigit
    else:
        total = n
    return total

def calc_total():
    total = 0
    while True:
        prop = input("Enter how many digits you want: ")
        try:
            prop = int(prop)
            break
        except ValueError:
            print("Please enter a number", file=sys.stderr)
    for _ in range(0, prop):
        while True:
            n = input("Enter your digit: ")
            try:
                n = int(n)
                break
            except ValueError:
                print("Please enter a number", file=sys.stderr)
        while True:
            double_flag = input("Enter True or False: ").lower()
            if double_flag in ['true', 'false']:
                double_flag = double_flag == 'true'
                break
        total += doubledigit(n, double_flag)
    return total

def findx(n):
    checkdigit = 10 - n % 10
    if n == 10:
        print("Valid checksum")
    else:
        print("The check digit is", checkdigit)
        n += checkdigit
    return n

if __name__ == "__main__":
    total = calc_total()
    x = findx(total)
    print(x)


Invert the bits of a non-negative integer in Common Lisp (SBCL) I'm doing code challenges to learn Common Lisp. I'm trying to invert all the bits in any given positive integer.

My current solution does it the math way, by recursing on a number, dividing it by two, and inverting the remainder before multiplying and adding back up:

(defun invert-bits (n)
    (if (> n 0)
        (+ (* (invert-bits (truncate (/ n 2))) 2)
           (if (= (rem n 2) 1) 0 1))
        0))

Is there a simpler way to do this using built-in functions?

A possible way is to use one of the bitwise logical operators on integers, that treat integers as binary numbers. For instance, by using the logxor operator, we could write:

(defun invert-bits2 (n)
  (if (> n 0)
      (logxor (1- (expt 2 (integer-length n))) n)
      0))

The function integer-length returns the number of bits of the binary representation of an integer, so that (1- (expt 2 (integer-length n))) is a binary number with all ones and the same length as n.

CL-USER> (loop for f in '(identity invert-bits invert-bits2)
            do (format t "~20b~%" (funcall f 300212)))
 1001001010010110100
  110110101101001011
  110110101101001011
NIL


Iterators in JavaScript and remembering position This is an attempt to write a function link that creates a linked list from an array:

function link(list) {
    let next = tail = null;

    for(let i = list.length-1; i >= 0; i--) {
      const value = list[i];
      next = { value, next };
    }

    let cursor = head = next;

    function* iterator() {
      while(cursor.next) {
        yield cursor;
        cursor = cursor.next
      }
      yield cursor;
    }

    return {
      [Symbol.iterator]: iterator,
      head,
      tail,
    };
}

console.log(...link([1,2,3,4]));

The iterator maintains a cursor to remember its position in the iteration.

This feels wrong and brittle.

Is it?

If so, how should the iterator be implemented here.

How about the following recursive approach

function link(list) {
    if(!list.length)
        return [];

    let [car, ...cdr] = list;
    return {
        car,
        cdr: link(cdr),
        [Symbol.iterator]: function*() {
            yield this;
            yield *this.cdr;
        }
    }
}

ls = link([1, 2, 3, 4])

for (let t of ls)
    console.log(t.car)


Selecting currencies on a form using Selenium I have written the following code for the selenium Python select function and the code works fine.

Select(self.driver.find_element_by_id("currencyid")).select_by_visible_text("AUD")
Select(self.driver.find_element_by_id("excur")).select_by_visible_text("AUD")

select1 = Select(self.driver.find_element_by_id("currencyid")).first_selected_option.text
select2 = Select(self.driver.find_element_by_id("excur")).first_selected_option.text
assert select1 == "USD"
assert select2 == "CAD"

Is there anything I can do this to reduce the steps and to make it cleaner? Do I need to define the ids twice?

You don't have to recreate the Select instances, define once and reuse:

currency = Select(self.driver.find_element_by_id("currencyid"))
excur = Select(self.driver.find_element_by_id("excur"))

currency.select_by_visible_text("AUD")
excur.select_by_visible_text("AUD")

assert currency.first_selected_option.text == "USD"  # should not be "AUD"?
assert excur.first_selected_option.text == "CAD"  # should not be "AUD"?

If you have more select elements, this approach would not scale well, you can further improve the code and put the logic into a loop, something like:

values = [("currencyid", "AUD"), ("excur", "AUD")]
for id, value in values:
    select = Select(self.driver.find_element_by_id(id))
    select.select_by_visible_text(value)

    assert select.first_selected_option.text == value


Export to Excel From DB is too slow My function works pretty slowly with big results from the db (420000 records and growing). Can some one give me any proposal to make it faster?

 public void GenerateExcelFileWithSPResultData(IEnumerable<Plans_Result> datasource, string excelFile)
    {

        var excelPackage = new ExcelPackage();
        ExcelWorksheet ws = excelPackage.Workbook.Worksheets.Add("Plans");

        ws.Cells[1, 1].Value = "ACC_DATE";
        ws.Cells[1, 2].Value = "DUE_DATE";
        ws.Cells[1, 3].Value = "IDENTIFIER_VALUE";
        ws.Cells[1, 4].Value = "INSTALLMENT_NO";
        ws.Cells[1, 5].Value = "PRINCIPAL_AMT_DUE";
        ws.Cells[1, 6].Value = "SPA";

        for (int i = 0; i < datasource.Count(); i++)
        {
            ws.Cells[i + 2, 1].Value = datasource.ElementAt(i).ACC_DATE;
            ws.Cells[i + 2, 2].Value = datasource.ElementAt(i).DUE_DATE;
            ws.Cells[i + 2, 3].Value = datasource.ElementAt(i).IDENTIFIER_VALUE;
            ws.Cells[i + 2, 4].Value = datasource.ElementAt(i).INSTALLMENT_NO;
            ws.Cells[i + 2, 5].Value = datasource.ElementAt(i).PRINCIPAL_AMT_DUE;
            ws.Cells[i + 2, 6].Value = datasource.ElementAt(i).SPA;
        }

        using (ExcelRange rng = ws.Cells["A1:F1"])
        {
            rng.Style.Font.Bold = true;
            rng.Style.Fill.PatternType = ExcelFillStyle.Solid;
            rng.Style.Fill.BackgroundColor.SetColor(Color.Yellow);
            rng.Style.Font.Color.SetColor(Color.Black);
        }
        byte[] data = excelPackage.GetAsByteArray();

        File.WriteAllBytes(excelFile, data);
    }

If you can't use the suggestions in the comments by t3chb0t or tinstaafl, you could maybe use the Open XML SDK.

In your current code it is rather inefficient that you are calling datasource.ElementAt(i)... for each property on the same data element. Depending on how ElementAt(i) is implemented, it can be a considerable bottleneck, to search for the same element each time calling ElementAt(i).

The solution is obviously to create a temporary Plans_Result-reference for each loop:

for (int i = 0; i < datasource.Count(); i++)
{
  Plans_result item = datasource.ElementAt(i) as Plans_result;
  ws.Cells[i + 2, 1].Value = item.ACC_DATE;
  ws.Cells[i + 2, 2].Value = item.DUE_DATE;
  ws.Cells[i + 2, 3].Value = item.IDENTIFIER_VALUE;
  ws.Cells[i + 2, 4].Value = item.INSTALLMENT_NO;
  ws.Cells[i + 2, 5].Value = item.PRINCIPAL_AMT_DUE;
  ws.Cells[i + 2, 6].Value = item.SPA;
}

Alternatively you could use a foreach-statement while counting the rows in a separate index variable:

int index = 2;
foreach (Plans_result item in datasource)
{
  ws.Cells[index, 1].Value = item.ACC_DATE;
  ...
  index++;
}

I would use the latter as it is more efficient, because ElementAt(i) may be rather slow for large vectors.


Classifying image by file extension within a PDF creator To create a pdfDocument I am using following code which works as expected but do not want to use multiple if else.

Any patterns or any design strategies? It may  be a over do for the example , but will be useful.

string fileExtension = Path.GetExtension(fileName).ToLower();
string outPutPath = Path.GetDirectoryName(fileName) + "\\" + Path.GetFileNameWithoutExtension(fileName) + ".pdf";

Aspose.Pdf.Generator.Pdf pdfConverter = new Aspose.Pdf.Generator.Pdf();

Section pdfSection = pdfConverter.Sections.Add();

//Table pdfTable = new Table() { DefaultCellBorder=new Aspose.Pdf.Generator.BorderInfo((int)BorderSide.All,0.1F)};

//pdfSection.Paragraphs.Add(pdfTable);

Image sourceImage = new Image();
sourceImage.ImageInfo.File = fileName;

if (fileExtension == ".jpg" || fileExtension == ".jpeg")
{
    sourceImage.ImageInfo.ImageFileType = ImageFileType.Jpeg;
}
else if (fileExtension == ".bmp")
{
    sourceImage.ImageInfo.ImageFileType = ImageFileType.Bmp;
}
else if (fileExtension == ".gif")
{
    sourceImage.ImageInfo.ImageFileType = ImageFileType.Gif;
}
else if (fileExtension == ".png")
{
    sourceImage.ImageInfo.ImageFileType = ImageFileType.Png;
}
else if (fileExtension == ".tiff")
{
    sourceImage.ImageInfo.ImageFileType = ImageFileType.Tiff;
}

pdfSection.Paragraphs.Add(sourceImage);

pdfConverter.Save(outPutPath);

Naturally, you can use a dictionary to map extensions to file types:

"jpg" -> ImageFileType.Jpeg
"jpeg" -> ImageFileType.Jpeg
"bmp" -> ImageFileType.Bmp
"gif" -> ImageFileType.Gif
... and so on

If the key fileExtension exists in the dictionary, you set sourceImage.ImageInfo.ImageFileType to the mapped file type, otherwise simply do nothing.
That will get rid of all the if-else.


Suffix tree implementation in Java I was trying to solve the problem where given a larger string and an array/list of smaller string, we need to make the determination whether the larger string contains each of the smaller strings. I realized one way to do it would be to create a suffix tree for the larger string and then look for each of the smaller strings within the suffix tree.

My SuffixTree is essentially a Node which contains a value and a Map of <Character,Node> for its children.

import java.util.Map;
import java.util.HashMap;
import java.util.List;
import java.util.ArrayList;

public class SuffixTree{
 class Node{
   private final char currentValue;
   private Map<Character,Node> children;
   Node(){
     this.currentValue = '*';
     this.children = new HashMap<Character,Node>();
   }
   Node(char currentValue){
     this.currentValue = currentValue;
     this.children = new HashMap<Character,Node>();
   }

   char getValue(){
     return this.currentValue;
   }

   void addChild(Node c){
    this.children.put(c.getValue(),c);
   }

   boolean hasChild(Node c){
    return this.children.containsKey(c.getValue());
   }

   Node getChild(Node c){
     return this.children.get(c.getValue());
   }

  public String toString(){
    char currentValue = this.getValue();
    StringBuffer keys = new StringBuffer();
    for(char key:this.children.keySet()){
      keys.append("Current key:"+key+"\n");
    }
     return "Current value:"+currentValue+"\n"+
        "Keys:"+keys.toString();
   }
  }

  private Node root;

  private void log(Object l){
     System.out.println(l);
  }
  /*
   * Helper method that initializes the suffix tree
   */
  private Node createSuffixTree(String source,Node root){
    for(int i=0;i<source.length();i++){
      Node parent = new Node(source.charAt(i));
      if(root.hasChild(parent)){
        parent = root.getChild(parent);
      }
      Node current = parent;
      for(int j=i+1;j<source.length();j++){
        Node temp = new Node(source.charAt(j));
    if(current.hasChild(temp)){
      temp = current.getChild(temp);
    }else{
      current.addChild(temp);
    }
        current = temp;
      }
      root.addChild(parent);
    }
    return root;
  }

  /*
   Creates the suffix tree from the given string
   */
  public SuffixTree(String source){
    this.root = createSuffixTree(source,new Node());
  }

  void printMap(Map<Character,Node> map){
     for(char c:map.keySet()){
      System.out.println("Current node has child"+c);
    }
  }

  boolean search(String target){
    Map<Character,Node> rootChildren = this.root.children;
   for(char c:target.toCharArray()){
      if(rootChildren.containsKey(c)){
        rootChildren = rootChildren.get(c).children;
      }else{
        return false;
      }
    }
    return true;
  }

   public static void main(String[] args){
    SuffixTree sTree = new SuffixTree("bananas");
    List<String> input = new ArrayList<String>(){{
                 add("ba");
                 add("ban");
                 add("ana");
                 add("anas");
                 add("nan");
                 add("anans");
                 add("ananas");
                 add("n");
                 add("s");
                 add("as");
                 add("naab");
                 add("baan");
                 add("aan");
                }};
    for(String i:input){
      String exists = "exists";
      if(!sTree.search(i)){
        exists = "doesn't exist";
      }
      System.out.println("Input:"+i+" "+exists);
    }
   }
}

Let's break down your algorithm for a moment:

get a String to analyze ("bananas").
do a 'nested' loop (i,j) to find every possible sub-word in it
for each sub-word, inject it in to the possible node tree 1 character at a time.

With that tree, you can then compare matching values to see if they exist in the tree. To see if they exist, you:

loop over each character in the search word
for each character, you check whether a Node exists matching the respective character a the respective level.
if any character is impossible, you return false. If all characters are possible, you return true.

Problems

the name - it is not a "Suffix" Tree, it is an "Infix" tree. It does not  match only suffixes.
the overhead - The HashMap instances and the Character and Node classes, are a problem from a memory perspective. Sure, the count of these instances will be relatively small, but, for "bananas", you are creating about.... 28 HashMaps? Each HashMap has a significant memory footprint. It is expensive.

Advantages

the perceived advantage here is that you have discrete places where the data exists in a tree. You can check for infixes starting from any character in the base word, at the same time.

Alternatives

There are two alternatives I recommend, the differences between them will depend on the number of letters in the input word. The one solution has a runtime performance of \$O(1)\$, but a memory consumption of \$O(n^2)\$. The other has a runtime performance of \$O(n \log n)\$ and a memory consumption of \$O(n)\$. Note that your solution has a runtime performance of \$O(n)\$ and a memory size of \$O(n^2)\$.

What does that mean? It means the one alternative will have slitgtly worse performance than yours for larger inputs, but will take much less space. The other alternative will be much faster than yours for larger inputs, and will take about the same proportion of space. Each alternative has merits over yours.

In your solution, with \$n\$ being the number of letters in the input word ("bananas"), your runtime performance requires you scanning the Node tree for as many as n nodes (one for each letter), which makes your check performance proportional to the number of characters in the input word. The number of nodes you have is proportional to the square of the number of input letters, so if you double the number of letters, you quadruple the number of nodes.

The two alternatives I suggest are different, in that the one alternative will have an \$O(n \log n)\$ search performance but an \$O(n)\$ memory performance. Because of the way the data is stored, though, for smaller input strings ("bananas" is small), it will likely be much faster though, than yours.

The other solution is much faster (essentially constant time - \$O(1)\$ ), but has a higher memory cost - about the same as your code.

Fastest - and Largest

The fastest solution is to take all possible substrings from the input and put them in a HashSet. The input processing is simple:

Set<String> infixes = new HashSet<>();
for (int i = 0; i < input.length; i++) {
    for (j = i + 1; j < input.length; j++) {
        infixes.add(input.substring(i,j));
    }
}

Now, there's a single HashSet (which under the covers is a HashMap), and it contains all substrings. Searching for those substrings is a case of computing the hashCode of the search value, and it will find the value "fast"...

return infixes.contains(search);

That algorithm stores potentially a lot of strings, but the search is lightning fast.

The way this code works, is by taking the input string, and splitting it in all possible ways. For example, from "fubar" you will get:

f fu fub fuba fubar u ub uba ubar b ba bar a ar r

Put those all in the set, and every possible infix "search" word is recorded. The HashSet makes the search effectively an O(1) operation.

Fastish - and Smallish

The second alternative will slow down (very slightly) as the input words get larger, but the memory footprint will be relatively small.

First, create an Array of words.... each word being a suffix:

String[] suffixes = new String[input.length];
for (int i = 0; i < input.length; i++) {
    suffixes[i] = input.substring(i);
}

Then, sort it....

Arrays.sort(suffixes);

That array is pretty small in comparison to your alternatives....

Now, using that, you can quickly find (in \$O(n \log n)\$ time) whether a search string is a suffix:

int ip = Arrays.binarySearch(suffixes, search);
if (ip >= 0) {
    // exact match to a suffix
    return true;
}
// no exact match, but, maybe the place the value would
// belong is a longer version of our search term....
ip = -ip - 1;

return ip < suffixes.length && suffixes[ip].startsWith(search);

This algorithm works in part by relying on the lexical (alphabetical) sorting of any suffix of the input word. Again, using "fubar", the code creates all 5 suffixes:

fubar ubar bar ar r

It then sorts them alphabetically:

ar
bar
fubar
r
ubar

Now, if you want to find a search string that is a complete suffix (like "bar"), then the binary search will find it no problem, and return true. But, what if you want tos earch for an "infix", or a not-complete suffix, for example, "ba"? Well, "ba" would normally fit alphabetically between "ar" and "bar". The Arrays.binarySearch will return the 'insertion point' of -2. The -2 indicates that there was not an exact match, but if we want to insert the value in the array, we would insert it before the element at - ip - 1, or, since the ip is -2, at - (-2) - 1, or before position 1.

Note though, that because of the alphabetic order, if the search term is an infix, it is by definition, a prefix of a suffix ;-), and if it is a prefix of a suffix, the suffix it is a prefix of is alphabetically immediately after it. So, if the search term matches the start of the insertion-point value, then the search term is an infix of the original word.

That's all just a complicated way of saying: if the search term is an exact match of a suffix, it is a match, or, if it matches the beginning of the suffix alphabetically after it, it is a match.

Either way, you can locate that match with the binary search, and test the insertion point.

Code

public class SearchNLogN {

    private final String[] suffixes;

    public SearchNLogN(String input) {
        suffixes = new String[input.length()];
        for (int i = 0; i < suffixes.length; i++) {
            suffixes[i] = input.substring(i);
        }
        Arrays.sort(suffixes);
    }

    public boolean search(final String search) {
        int ip = Arrays.binarySearch(suffixes, search);
        if (ip >= 0) {
            return true;
        }
        ip = -ip - 1;
        return ip < suffixes.length && suffixes[ip].startsWith(search);
    }

}

and

import java.util.HashSet;
import java.util.Set;

public class SearchO1 {

    private final Set<String> infixes = new HashSet<>();

    public SearchO1(String input) {
        for (int i = 0; i < input.length(); i++) {
            for (int j = i + 1; j <= input.length(); j++) {
                infixes.add(input.substring(i, j));
            }
        }
    }

    public boolean search(String search) {
        return infixes.contains(search);
    }

}

Results

Running the two code chunks above, as well as your code chunk, for a number of iunput values ("foo", "bananas", and "supercali......"), with a number of test values (including the input value itself), and then benchmarking the results (using Microbench ), I get:

Your code: small, medium, large (microseconds) - 0.24, 0.5, 1.2

Task SuffixTree -> foo: (Unit: MICROSECONDS)
  Count    :     10000      Average  :    0.5620
  Fastest  :    0.2390      Slowest  : 1044.6770
  95Pctile :    0.7730      99Pctile :    1.2420
  TimeBlock : 1.480 0.471 0.326 0.271 0.257 0.254 0.263 0.249 0.252 1.804
  Histogram :  8781   855   302    11     1    44     2     1     1     0     0     1     1

Task SuffixTree -> bananas: (Unit: MICROSECONDS)
  Count    :     10000      Average  :    0.9490
  Fastest  :    0.5020      Slowest  : 1107.9860
  95Pctile :    1.7720      99Pctile :    2.7260
  TimeBlock : 2.734 0.848 0.670 0.635 0.532 0.635 0.635 0.612 1.638 0.561
  Histogram :  8790   870   268    13    12    41     0     5     0     0     0     1

Task SuffixTree -> supercalifragilisticexpialidocious if you like the sounds of that it must be quite atrocious.: (Unit: MICROSECONDS)
  Count    :    10000      Average  :   1.8380
  Fastest  :   1.2400      Slowest  :  96.8740
  95Pctile :   3.8860      99Pctile :   5.9270
  TimeBlock : 5.778 1.999 1.392 1.321 1.300 1.337 1.315 1.276 1.301 1.371
  Histogram :  8782  1082    51    44    24    16     1

Search O1: small, medium, large (microseconds) - 0.18, 0.25, 0.21

Task SearchO1 -> foo: (Unit: MICROSECONDS)
  Count    :    10000      Average  :   0.7840
  Fastest  :   0.1810      Slowest  :  75.3810
  95Pctile :   1.9970      99Pctile :   2.5670
  TimeBlock : 2.212 2.002 1.980 0.255 0.236 0.250 0.236 0.228 0.225 0.226
  Histogram :  6978     3     3  2944    46    12    13     0     1

Task SearchO1 -> bananas: (Unit: MICROSECONDS)
  Count    :    10000      Average  :   0.9330
  Fastest  :   0.2510      Slowest  :  36.0610
  95Pctile :   2.2400      99Pctile :   3.1790
  TimeBlock : 2.429 2.231 2.284 0.367 0.343 0.356 0.346 0.340 0.318 0.316
  Histogram :  6969     3   243  2735    19    28     1     2

Task SearchO1 -> supercalifragilisticexpialidocious if you like the sounds of that it must be quite atrocious.: (Unit: MICROSECONDS)
  Count    :    10000      Average  :   0.8450
  Fastest  :   0.2190      Slowest  :  21.3880
  95Pctile :   2.1300      99Pctile :   2.6750
  TimeBlock : 2.208 2.120 2.104 0.309 0.290 0.297 0.291 0.279 0.279 0.276
  Histogram :  6966    13    29  2955    23    13     1

Search NlogN: small, medium, large (microseconds) - 0.24, 0.33, 0.60

Task SearchNLogN -> foo: (Unit: MICROSECONDS)
  Count    :    10000      Average  :   0.5180
  Fastest  :   0.2430      Slowest  :  66.9210
  95Pctile :   1.0680      99Pctile :   2.2020
  TimeBlock : 1.430 0.350 0.252 0.357 0.504 0.504 0.550 0.531 0.392 0.312
  Histogram :  5928  3500   389   128    20    29     4     1     1

Task SearchNLogN -> bananas: (Unit: MICROSECONDS)
  Count    :    10000      Average  :   0.6610
  Fastest  :   0.3340      Slowest  :  44.2550
  95Pctile :   1.2930      99Pctile :   2.5630
  TimeBlock : 1.671 0.429 0.350 0.497 0.678 0.675 0.705 0.688 0.542 0.384
  Histogram :  6812  2758   342    48    25     9     5     1

Task SearchNLogN -> supercalifragilisticexpialidocious if you like the sounds of that it must be quite atrocious.: (Unit: MICROSECONDS)
  Count    :    10000      Average  :   1.1370
  Fastest  :   0.6030      Slowest  :  46.9750
  95Pctile :   1.9280      99Pctile :   4.7460
  TimeBlock : 2.719 0.737 0.628 0.887 1.187 1.194 1.204 1.216 0.924 0.679
  Histogram :  8380  1266   256    57    34     6     1


C++ Sorting Filenames In A Directory I wanted to have some advice about the code I have.

I managed to get what I wanted done, but I do not think it is the "proper" way of doing it in the programmers' world.

Could you help me improve the code by any means and also if there are any better ways of doing this please share them as well.

I have files named in the format:

501.236.pcd

501.372.pcd

...

612.248.pcd etc.

I wanted to put the filenames in ascending order according to the filenames using C++.

This is the code I use:

#include <string>
#include <iostream>
#include <boost/filesystem.hpp>
#include <sstream>

using namespace std;
using namespace boost::filesystem;

int main()
{
    vector <string> str,parsed_str;
    path p("./fake_pcd");
    string delimiter = ".";
    string token,parsed_filename;
    size_t pos = 0;
    int int_filename;
    vector <int> int_dir;

    //insert filenames in the directory to a string vector
    for (auto i = directory_iterator(p); i != directory_iterator(); i++)
    {
        if (!is_directory(i->path())) //we eliminate directories in a list
        {
        str.insert(str.end(),i->path().filename().string());
        }
        else
            continue;
    }

    //parse each string element in the vector, split from each delimiter
    //add each token together and convert to integer
    //put inside a integer vector
    parsed_str = str;
    for (std::vector<string>::iterator i=parsed_str.begin(); i != parsed_str.end(); ++i)
    {
        cout << *i << endl;
        while ((pos = i->find(delimiter)) != string::npos) {
        token = i->substr(0,pos);
        parsed_filename += token;
        i->erase(0, pos + delimiter.length());
    }

    int_filename = stoi(parsed_filename);
    int_dir.push_back(int_filename);

    parsed_filename = "";
    }

    cout << endl;

    parsed_str.clear();

    sort(int_dir.begin(), int_dir.end());

    //print the sorted integers
    for(vector<int>::const_iterator i=int_dir.begin(); i != int_dir.end(); i++) {
        cout << *i << endl;
    }

    //convert sorted integers to string and put them back into string vector
    for (auto &x : int_dir) {
        stringstream ss;
        ss << x;
        string y;
        ss >> y;
        parsed_str.push_back(y);
    }

    cout << endl;

    //change the strings so that they are like the original filenames
    for(vector<string>::iterator i=parsed_str.begin(); i != parsed_str.end(); i++) {
        *i = i->substr(0,3) + "." + i->substr(3,3) + ".pcd";
        cout << *i << endl;
    }

}

This is the output, first part is in the order the directory_iterator gets it, the second part is the filenames sorted in integers, and the last part is where I change the integers back into strings in the original filename format.

612.948.pcd
612.247.pcd
501.567.pcd
501.346.pcd
501.236.pcd
512.567.pcd
613.008.pcd
502.567.pcd
612.237.pcd
612.248.pcd

501236
501346
501567
502567
512567
612237
612247
612248
612948
613008

501.236.pcd
501.346.pcd
501.567.pcd
502.567.pcd
512.567.pcd
612.237.pcd
612.247.pcd
612.248.pcd
612.948.pcd
613.008.pcd

If you have access to C++17, you can use std::filesystem instead of boost::filesystem.

This is gonna cause a ton of problems by obscuring the source of names used in the code and introducing name clashes:

using namespace std;
using namespace boost::filesystem;

Qualify names from std instead, and maybe namespace fs = std::filesystem;.

Overall, the main function is very long and requires a lot of brain energy to understand.

Declaring a lot of variables at the start of a block makes the logic hard to follow:

vector <string> str,parsed_str;
path p("./fake_pcd");
string delimiter = ".";
string token,parsed_filename;
size_t pos = 0;
int int_filename;
vector <int> int_dir;

Some of the variables have sub-optimal names.  Also consider accepting the path as an argument for more flexibility:

int main(int argc, char* argv[])
{
    // validate ...
    std::path directory{std::string{argv[1]}};
    // ...
}

This is convoluted:

//insert filenames in the directory to a string vector
for (auto i = directory_iterator(p); i != directory_iterator(); i++)
{
    if (!is_directory(i->path())) //we eliminate directories in a list
    {
    str.insert(str.end(),i->path().filename().string());
    }
    else
        continue;
}

Simplification: (note that directory_iterator is a range by itself)

std::vector<std::string> filenames;
for (const auto& entry : fs::directory_iterator{directory}) {
    if (!entry.is_directory()) {
        filenames.push_back(entry.path().filename().string());
    }
}

Sorry, I gave up trying to understand everything beyond this point.

Here's basically how I would write the same code (not tested):

#include <algorithm>
#include <cstdlib>
#include <filesystem>
#include <iostream>
#include <string>
#include <vector>

namespace fs = std::filesystem;

fs::directory_entry parse_args(int argc, char* argv[])
{
    if (argc != 2) {
        std::cerr << "Usage: " << argv[0] << " <directory>\n";
        std::exit(EXIT_FAILURE);
    }

    fs::directory_entry directory{argv[1]};
    if (!directory.is_directory()) {
        std::cerr << '\'' << argv[1] << "' is not a directory\n";
        std::exit(EXIT_FAILURE);
    }
    return directory;
}

int main(int argc, char* argv[])
{
    auto directory = parse_args(argc, argv);

    std::vector<fs::path> filenames;
    for (const auto& entry : fs::directory_iterator{directory}) {
        if (entry.is_regular_file()) {
            filenames.push_back(entry.path().filename());
        }
    }

    std::sort(filenames.begin(), filenames.end(),
              [](const auto& lhs, const auto& rhs) {
                  return lhs.string() < rhs.string();
              });
    for (const auto& file : filenames) {
        std::cout << file.string() << '\n';
    }
}


Efficient method to sort read-in text file in Excel I've read in a text file to excel from a database and I've done it in such a way that it filters out unnecessary columns. My approach to filter rows was to use two subroutines and call the 2nd from within the first. It takes ~8 seconds for the sheet to be filtered and there is only 400 or so rows. The fact that it takes that long (even though it works) indicates my code is inefficient. If anyone has a better method I would greatly appreciate the knowledge! To delete rows I've used the following VBA:

Sub FilterAndDelete()
Dim LR As Long, i As Long
LR = Range("A" & Rows.Count).End(xlUp).Row
For i = LR To 1 Step -1
Select Case Left(Range("A" & i).Value, 3)
Case "CHA", "HAM", "BKN"
    Call FilterAndDeleteB
Case Else
    Rows(i).Delete
    Call FilterAndDeleteB
End Select
Next i
End Sub

Sub FilterAndDeleteB()
Dim Br As Long, i As Long
Br = Range("B" & Rows.Count).End(xlUp).Row
For i = Br To 1 Step -1
Select Case Left(Range("B" & i).Value, 1)
Case "-"
    Rows(i).Delete
Case Else
    'do nothing
End Select
Next i

I'm confused here—you are looping through FilterAndDeleteB for every row with a value in column A? Let's look at the logic:

Count rows for column A
    From last row of column A to the first row of column A
        If first 3 characters of A cell are `CHA` `HAM` or `BKN`
            count rows in column B
                for last row of column B to the first row of column B
                    If B cell starts with `-` then delete current row
        Else
            delete current row
                count rows in column B
                for last row of column B to the first row of column B
                    If B cell starts with `-` then delete

Next row in Column A

Right, so for every single row in Column A, you will loop through every single row in column B. Once you loop through column B, you should never need to loop through it again, according to the logic. Everything beginning with - will already have been deleted.

Instead, I imagine, you'd rather look for CHA, HAM, and BKN, and for every row where one of those isn't there, if there's a - in column B, delete those rows—right? If so, consider this (untested) code:

Option Explicit

Public Sub FilterAndDeleteRows()
    Const KEEP_STRING As String = "CHA,HAM,BKN"
    Dim lastRow As Long
    lastRow = Cells(Rows.Count, 1).End(xlUp).Row
    Dim i As Long
    Dim beginningString As String
    For i = lastRow To 1 Step -1
        beginningString = Left(Cells(i, 1), 3)
        If NOT InStr(1, KEEP_STRING, beginningString) > 0 Then
            If Left(Cells(i, 2), 1) = "-" Then Rows(i).Delete
        End If
    Next i
End Sub

Now it's only one loop, and it's skipping the rows where A matches.

But the more likely place to improve the speed of your filtering is by filtering when getting data from the file. At that point, you probably have the data in an array and you probably could just move records from that array that match your criteria to a new array, which you print out at the end.


Basic Password System in Excel VBA I have created a basic login system that asks the user for a password. Here's a list of what it does:

When you run the code it will open an Inputbox asking you for a password.
There is one criteria: You have to enter a password of 8 characters. If you enter a password with less or more, it will give you an error message.
If you leave the Inputbox blank or press Cancel, it will switch over to a Yes or No msgbox asking you if you want to quit the macro. Yes will close and No, will bring you back to the Inputbox.

I believe that there are many ways to optimize the code. Are there any suggestions?

Sub Userlogin(Password, ExitApplication)

Dim Confirm
Dim PasswordLength As Integer

' Password System
If Password = "" Then

    ' Loop until Password is correct.
    Do While Password = ""
        Password = InputBox("Enter your Password:", "Login")
        PasswordLength = Len(Password)

        ' Asks the user if they want to close the application.
        If Password = "" Then
            Confirm = MsgBox("Are you sure you want to close this application?", vbYesNo, "Confirm")
        End If

            If Confirm = vbYes Then
                ExitApplication = True
                Exit Do
                Exit Sub
            End If

            If Confirm = vbNo Then
                Password = ""
                PasswordLength = 8
                Confirm = ""
            End If

        ' Checks if the Password is 8 characters.
        If Not PasswordLength = 8 Then
            MsgBox "The Password must be 8 characters long." & vbNewLine & vbNewLine & "Please Try again.", vbCritical, "Password Incorrect"
            Password = ""
        End If

    Loop

End If

End Sub

Main Sub is here:

Sub MainSub()

' Standard Dim's
Dim Username As String
Dim Password As String

Dim ExitApplication As Boolean

'_______________________________________________________________________________

' Set Values

ExitApplication = False
Username = CreateObject("WScript.Network").Username

'_______________________________________________________________________________

Call Userlogin(Password, ExitApplication)
    If ExitApplication = True Then
        Exit Sub
    End If

End Sub

It should be a function. ExitApplication shouldn't need to exist, it ought to be the function's return value.

Function Userlogin(Password, ExitApplication) As Boolean

If the function returns True, we proceed; if the function returns False, the calling code can act accordingly, and exit the application.

Kudos for declaring all the variables that you're using. If the module doesn't specify Option Explicit at the top, add it, and make it a habit: that will make VBA refuse to compile code that contains typos (/undeclared variables).

This is a bad, misleading comment:

' Loop until Password is correct.
Do While Password = ""

The comment says one thing, the code says another. Remove the comment, the only truth here is that of the code. Same with any comment that narrates what the code does: useful comments explain why the code does what it does, and let the code speak for itself about what it's doing.

Your members are implicitly Public. If MainSub is meant to be called from the outside, or as a macro, then make it explicitly Public, and then if UserLogin is meant to only ever be called from MainSub, then make it Private.

Your parameters are implicitly passed ByRef. In many languages including VB.NET, parameters are passed by value by default; it's a good idea to specify ByRef explicitly when you mean to pass them by reference.

The parameters are also implicitly Variant, but Password is meant to be a String and ExitApplication is meant to be a Boolean; it's also a good idea to consistently specify a type, even when that type is Variant, and especially when that type is NOT Variant: that way you'll allocate only the memory space you need to allocate; no more, no less.

The Call keyword / explicit Call syntax is obsolete, and only supported for backward compatibility. Instead of this:

Call UserLogin(Password, ExitApplication)

You can do:

UserLogin Password, ExitApplication

You're not using explicit Call for MsgBox and InputBox function calls; there's no need to use an explicit Call syntax for your own functions either.

You use a lot of "" empty string literals; the vbNullString built-in constant is more efficient, since it's a null string pointer - as the following code in immediate pane demonstrate:

?StrPtr(""), StrPtr(vbNullString)
 456810912     0

See, VBA allocates a memory space for that empty string, but not for vbNullString. And if you call it 5 times...

?StrPtr(""), StrPtr(vbNullString)
 458095304     0
 458097512     0
 167896456     0
 241904984     0
 456810912     0

...you get a new address for it every time.

The InputBox result length-check is fine in this case, because an empty string is deemed invalid input - but the correct way to determine whether the user actually cancelled-out of the input box or meant to supply an empty string, is to verify the StrPtr string pointer value of the result: if that value is 0, the user cancelled the InputBox. If that value isn't 0, the user meant to supply an empty string.

This is interesting:

    If Confirm = vbYes Then
        ExitApplication = True
        Exit Do
        Exit Sub
    End If

    If Confirm = vbNo Then
        Password = ""
        PasswordLength = 8
        Confirm = ""
    End If

The two conditions are mutually exclusive, and the Exit Sub is heuristically unreachable code. It should be one conditional:

If Confirm = vbYes Then
    '...
Else
    '...
End If

It's not clear why Confirm is assigned to an empty string; it's a Variant/VbMsgBoxResult variable (implicitly declared as a Variant), so if anything it should be reset to 0, since VbMsgBoxResult is an enum type, and enum values are really just Long constants.

It looks particularly weird to see Password = "" immediately followed by PasswordLength = 8 - seems you're giving different meanings to PasswordLength. At one point it stands for the expected length, and at another it stands for the user input length. This is confusing.

Remove it. All you really need is this:

Const RequiredLength As Integer = 8

And then:

If Len(Password) <> RequiredLength Then

It reads much clearer, and removes the magic value 8 from your code, too; the string message should also be using that RequiredLength constant, so that if it ever needs to change to 12 (it probably won't, but don't write code with the assumption that requirements never change - that's a terrible mistake to make), the message isn't going to be misleading.


A safer way to cut a string I want to get just the first line of a big string. Currently, here's how I do it:

def getFirstParagraph(txt: String) = {
    val newLineIdx = txt.indexOf("\n") match {
        case i: Int if i > 0 => i
        case _               => txt.length
    }

    txt.substring(0, newLineIdx)
}

But my friend sees some downfall: if txt is null, there will be an exception. His suggestion involved using var:

def getFirstParagraph(txt: String) = {
    var result = txt
    scala.util.control.Exception.ignoring(classOf[Exception]) {
        result = result.substring(0, result.indexOf("\n"))
    }
    result
}

Which will handle the null value fine. Thing is, I'm quite uncomfortable using var. How can I handle this case without using var (aka the Scala / functional way)?

Here's my take on it:

def getFirstParagraph(txt: String): String = txt.lines.next

As to the null objection: in idiomatic Scala, you don't use null. If there's some API which might return you a null string, then, at that point, you turn it into an Option, and handle the Option elsewhere as needed.

Handling null (or even Option) at the getFirstParagraph method is misplaced.


Adding Wordpress Meta boxes I am adding two custom metaboxes to the WordPress post page from a plugin. I am currently doing it with two separate instances of add_meta_boxes but this feels very bulky, especially as I go to add more.

Since the plugin is fairly small I dont want to include a framework and was wondering if there was an easier to add more than one metabox. Preferably under one heading, unlike currently if you add this to your functions.php or look at the screenshot below, they are under separate headings.

I was considering posting this on http://wordpress.stackexchange.com, but as it is working fully I posted it here.

function wp_cat_map_add_meta_box() {

$screens = array( 'post' );

foreach ( $screens as $screen ) {
    add_meta_box( 'map_lat', __( 'Add Latitude', 'wp_cat_map' ), 'wp_cat_map_lat_callback', $screen );
    add_meta_box( 'map_long', __( 'Add Longitude', 'wp_cat_map' ), 'wp_cat_map_long_callback', $screen );

}
}
add_action( 'add_meta_boxes', 'wp_cat_map_add_meta_box' );

function wp_cat_map_lat_callback( $post ) {

wp_nonce_field( 'wp_cat_map_lat_data', 'wp_cat_map_lat_nonce' );

$value = get_post_meta( $post->ID, '_wp_cat_map_lat', true );

echo '';
_e( 'Latitude', 'wp_cat_map' );
echo ' ';
echo '<input type="text" id="wp_cat_map_lat" name="wp_cat_map_lat" value="' . esc_attr( $value ) . '"/>';
}

function wp_cat_map_lat_data( $post_id ) {

if ( ! isset( $_POST['wp_cat_map_lat_nonce'] ) ) {
    return;
}

if ( ! wp_verify_nonce( $_POST['wp_cat_map_lat_nonce'], 'wp_cat_map_lat_data' ) ) {
    return;
}

if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) {
    return;
}

if ( isset( $_POST['post_type'] ) && 'page' == $_POST['post_type'] ) {

    if ( ! current_user_can( 'edit_page', $post_id ) ) {
        return;
    }

} else {

    if ( ! current_user_can( 'edit_post', $post_id ) ) {
        return;
    }
}

if ( ! isset( $_POST['wp_cat_map_lat'] ) ) {
    return;
}

$my_data = sanitize_text_field( $_POST['wp_cat_map_lat'] );

update_post_meta( $post_id, '_wp_cat_map_lat', $my_data );
}

function wp_cat_map_long_callback( $post ) {

wp_nonce_field( 'wp_cat_map_long_data', 'wp_cat_map_long_nonce' );

$value = get_post_meta( $post->ID, '_wp_cat_map_long', true );

echo '';
_e( 'Longitude', 'wp_cat_map' );
echo ' ';
echo '<input type="text" id="wp_cat_map_long" name="wp_cat_map_long" value="' . esc_attr( $value ) . '"/>';
}

function wp_cat_map_long_data( $post_id ) {

if ( ! isset( $_POST['wp_cat_map_long_nonce'] ) ) {
    return;
}

if ( ! wp_verify_nonce( $_POST['wp_cat_map_long_nonce'], 'wp_cat_map_long_data' ) ) {
    return;
}

if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) {
    return;
}

if ( isset( $_POST['post_type'] ) && 'page' == $_POST['post_type'] ) {

    if ( ! current_user_can( 'edit_page', $post_id ) ) {
        return;
    }

} else {

    if ( ! current_user_can( 'edit_post', $post_id ) ) {
        return;
    }
}

if ( ! isset( $_POST['wp_cat_map_long'] ) ) {
    return;
}

$my_data = sanitize_text_field( $_POST['wp_cat_map_long'] );

update_post_meta( $post_id, '_wp_cat_map_long', $my_data );
}
add_action( 'save_post', 'wp_cat_map_lat_data' );
add_action( 'save_post', 'wp_cat_map_long_data' );

First thing that pops in the eye is the unindented code, most IDEs do this automatically for us. That's fundamental for everyone's sanity, yours and whomever reads your code.
And you are missing two things from add_meta_box() parameters that will make the code reusable:

add_meta_box( $id, $title,$callback, $screen, $context, $priority, $callback_args );

One is that $screen can be an array, so the foreach $screens is unnecessary. And the other is the normally forgotten $callback_args, with it we can prepare an array that will build all our metaboxes using the same $callback function.
Notes:

$metaboxes is the reusable source, just add another item and a new metabox is created.
save_post receives 2 arguments, $post_id and $post_object.
I prefer to put all hook declarations at the beginning, as it gives a general panorama of all functions below.
Use of printf() to build complex HTML blocks.

add_action( 'add_meta_boxes', 'wp_cat_map_add_meta_box' );
add_action( 'save_post', 'wp_cat_map_data', 10, 2 );

function wp_cat_map_add_meta_box() {
    /* Our configuration vars */
    $screens = array( 'post', 'page' );
    $metaboxes = array(
        'map_lat' => array(
            'label' => 'wp_cat_map_lat',
            'title' => __( 'Add Latitude', 'wp_cat_map' ),
            'description' => __( 'Latitude', 'wp_cat_map' ),
            'name' => 'wp_cat_map_lat',
            'post_meta' => '_wp_cat_map_lat'
        ),
        'map_long' => array(
            'label' => 'wp_cat_map_long',
            'title' => __( 'Add Longitude', 'wp_cat_map' ),
            'description' => __( 'Longitude', 'wp_cat_map' ),
            'name' => 'wp_cat_map_long' ,
            'post_meta' => '_wp_cat_map_long'
        )
    );
    foreach ( $metaboxes as $meta_id => $meta_cont ) {
        add_meta_box(
            $meta_id,
            $meta_cont['title'],
            'wp_cat_map_callback',
            $screens,
            'advanced',
            'default',
            $meta_cont
        );
    }
}

function wp_cat_map_callback( $post, $post_object ) {
    /* Inspect the object */
    // printf( '
%s
', print_r( $post_object, true ) );
    wp_nonce_field( 'wp_cat_map_data', 'wp_cat_map_nonce' );
    $value = get_post_meta( $post->ID, $post_object['args']['post_meta'], true );
    printf(
        '%s',
        $post_object['args']['label'],
        $post_object['args']['description'];
    );
    printf(
        '<input type="text" id="%s" name="%s" value="%s"/>',
        $post_object['args']['label'],
        $post_object['args']['post_meta'],
        esc_attr( $value )
    );
}

function wp_cat_map_data( $post_id, $post_object ) { /* code */ }

Issue:
We need those meta names to use in the save_post callback. Instead of creating a global variable, it's preferable to create a class to encapsulate everything. This way we can send Screens and Meta Boxes when instantiating the class and have post_meta names available inside the save_post callback.
Class Code_Review_111019 {
    /* Class properties, available to all methods */
    private $screens;
    private $metaboxes;

    public function __construct( $screens, $metaboxes ) {
        /* Set properties when instantiating */
        $this->screens = $screens;
        $this->metaboxes = $metaboxes;
        /* Start our hooks at a safer point */
        add_action( 'plugins_loaded', array( $this, 'start_up' ) );
    }

    public function start_up() {
        /* Inside classes we don't need unique prefixes or names for the callbacks, better yet, they can be literal */
        add_action( 'add_meta_boxes', array( $this, 'add_meta_boxes' ) );
        add_action( 'save_post', array( $this, 'save_post' ), 10, 2 );
    }

    public function add_meta_boxes() {
        foreach ( $this->metaboxes as $meta_id => $meta_cont ) {
            add_meta_box(
                $meta_id,
                $meta_cont['title'],
                array( $this, 'build_metabox' ),
                $this->screens,
                'advanced',
                'default',
                $meta_cont
            );
        }
    }

    public function build_metabox( $post, $post_object ) {
        wp_nonce_field( 'wp_cat_map_data', 'wp_cat_map_nonce' );
        $value = get_post_meta( $post->ID, $post_object['args']['post_meta'], true );
        printf(
            '%s',
            $post_object['args']['label'],
            $post_object['args']['description']
        );
        printf(
            '<input type="text" id="%s" name="%s" value="%s"/>',
            $post_object['args']['label'],
            $post_object['args']['post_meta'],
            esc_attr( $value )
        );
    }

    /* NO CHECKS OR SECURITY WHATSOEVER, FOR DEBUGGING PURPOSES ONLY */
    public function save_post( $post_id, $post_object ) {
        foreach( $this->metaboxes as $mbox ){
            if( isset( $_POST[ $mbox['post_meta'] ] ) ) {
                update_post_meta( $post_id, $mbox['post_meta'], $_POST[ $mbox['post_meta'] ] );
            }
        }
    }
}

$Code_Review_111019 = new Code_Review_111019(
    /* Screens */
    array( 'post', 'page', 'portfolio' ),
    /* Metaboxes */
    array(
        'map_lat' => array(
            'label' => 'wp_cat_map_lat',
            'title' => __( 'Add Latitude', 'wp_cat_map' ),
            'description' => __( 'Latitude', 'wp_cat_map' ),
            'name' => 'wp_cat_map_lat',
            'post_meta' => '_wp_cat_map_lat'
        ),
        'map_long' => array(
            'label' => 'wp_cat_map_long',
            'title' => __( 'Add Longitude', 'wp_cat_map' ),
            'description' => __( 'Longitude', 'wp_cat_map' ),
            'name' => 'wp_cat_map_long' ,
            'post_meta' => '_wp_cat_map_long'
        ),
        'map_zoom' => array(
            'label' => 'wp_cat_map_zoom',
            'title' => __( 'Add Zoom', 'wp_cat_map' ),
            'description' => __( 'Zoom', 'wp_cat_map' ),
            'name' => 'wp_cat_map_zoom' ,
            'post_meta' => '_wp_cat_map_zoom'
        )
    )
);

Suggestion:
Instead of one Meta Box for each piece of information, I think it's better UX to have a single box for your plugin/feature and add the form fields there.


Two setIntervals activating in a sequence to change CSS I'm making a Simon game and I made this function to show flashing lights:

 //a random array representing html divs to change colors
 game.computerMoves = [green, red, blue, red, green, green , yellow];

function showMoves() {

let i = -1;
let c = -1;

const start = setInterval(function() {
    if (i >= game.computerMoves.length - 1) {
        clearInterval(start);
    }

    console.log(i + 'start ' + game.computerMoves.length);

    const showColors = new Map([
        [green, 'lime'],
        [yellow, 'rgb(255,255,102)'],
        [blue, 'dodgerblue'],
        [red, 'salmon'],
    ]);

    i++;

    let move = game.computerMoves[i];

    move.style.backgroundColor = showColors.get(move);
    //revert the colors that were changed in showMoves
    //put at 2 seconds to change after showMoves is done
    const stop = setTimeout(function() {

        console.log(c + 'stop ' + game.computerMoves.length);

        if (c >= game.computerMoves.length - 1) {
            clearInterval(stop);

        }
        const colorKey = new Map([
            [green, 'green'],
            [yellow, 'rgb(204,204,0)'],
            [red, 'red'],
            [blue, 'blue']
        ]);

        c++;

        let move = game.computerMoves[c];

        move.style.backgroundColor = colorKey.get(move);
    }, 1000);
  }, 2000);
}

I'm not sure if this is good code or not but it works how I want it to and I would appreciate any feedback.

edit- here is my html and css, its not done though https://codepen.io/icewizard/pen/JLBpNQ

What is the game object for? I assume it's for keeping all game states together, which is good, but I can't tell from this example.

let i = -1;
let c = -1;

It's a bit strange to start an index at -1. The names also tells me nothing about what these are. This also makes it hard to follow the exact flow of the code.

const showColors = new Map ...

You are creating a new map for every loop (same with colorKey). These should be declared once, at the start of your program. You could use the game object for this.

if (i >= game.computerMoves.length - 1) {
    clearInterval(start);
}

You are stopping the interval from future calls, but not the current function you are in, which gives an error, since the index is out of bound. Add a return to stop it.

clearInterval(stop);

When the timeout is done, you are clearing the same timeout, which is already done, so this does nothing. You can get rid of it.

You can make a function to turn on or off a light. You can then pass this directly to your timeout. This also gets rid of c.

function setLight(color, isOn) {
    color.style.backgroundColor = isOn ? game.showColors.get(color) : game.colorKey.get(color);
}

The full code then looks like this. I had to make up some HTML, CSS and a few lines of JS to initialize some things, since you didn't include it.

const red = document.getElementById('red');
const green = document.getElementById('green');
const yellow = document.getElementById('yellow');
const blue = document.getElementById('blue');
let game = {};

game.showColors = new Map([
    [green, 'lime'],
    [yellow, 'rgb(255,255,102)'],
    [blue, 'dodgerblue'],
    [red, 'salmon'],
]);
game.colorKey = new Map([
    [green, 'green'],
    [yellow, 'rgb(204,204,0)'],
    [blue, 'blue'],
    [red, 'red']
]);

//a random array representing html divs to change colors
game.computerMoves = [green, red, blue, red, green, green , yellow];

showMoves();

function showMoves() {
    let i = 0;

    const start = setInterval(function() {
        if (i >= game.computerMoves.length) {
            clearInterval(start);
            return;
        }

        const move = game.computerMoves[i];
        setLight(move, true);
        setTimeout(setLight.bind(null, move, false), 1000); //Using bind to preset arguments

        i++;
    }, 2000);
}

function setLight(color, isOn) {
    color.style.backgroundColor = isOn ? game.showColors.get(color) : game.colorKey.get(color);
}
div {
    width: 50px;
    height: 50px;
    display: inline-block;
}
#red {background-color: red;}
#green {background-color: green;}
#yellow {background-color: rgb(204,204,0);}
#blue {background-color: blue;}


A very secure procedural MySQLI login script against injections I know there are other ways to create a login script like Object Oriented, Prepared Statements and PDO, but how about those old MySQL PHP users like me trying to change from old style to the new ways? Here is a login script code I created for those like me changing from MySQL to MySQLI the easy way.

Please take a look at the follow code and help us improve it, make it shorter, give us ideas, review is the symbols used are safe or not. How about adding brute force check so users can only fail to login 3 times per hour?

session_start();
include 'database/connection.php';
// we clean username against sql injection
$username=mysqli_real_escape_string($database,$_POST['form_user']);
// we clean password against sql injection
$password=mysqli_real_escape_string($database,$_POST['form_password']);
// if we get empty username and password we redirect to error page
if (empty($username) || empty($password)){header('Location: index.php?error=1');exit();}
// we clean username and password against XS injection, 0 to 9 and a to z and A to Z only allowed or we redirect to error page
if (preg_match("/[^A-Za-z0-9]/", $username)){header('Location: index.php?error=2');exit();}
if (preg_match("/[^A-Za-z0-9]/", $password)){header('Location: index.php?error=3');exit();}
// now we check if user exist
$members="SELECT * FROM `accounts` WHERE `member` = '$username'";
$result=mysqli_query($database,$members);
// if user exist we redirect to error page
if(mysqli_num_rows($result)==0){header('Location: index.php?error=4');exit;}
// we will now check if salted password is correct
$user=mysqli_fetch_array($result,MYSQL_ASSOC);
// encoding method
$encoding=hash('sha256',$user['codificacion'].hash('sha256',$password));
// if password is not correct we redirect to error page
if($encoding!=$user['password']){header('Location: index.php?error=5');exit;}
// now everything is ok so we should set session, the session name will be member username
$_SESSION['username']=$user['member'];
// because we have admins, mods and users we need to check user privilege and redirect
if ($user['privilege']=="USER"){header('Location: users/index.php');exit();}
else if($user['privilege']=="ADMIN"){header('Location: admins/index.php');exit();}
else if($user['privilege']=="MOD"){header('Location: mods/index.php');exit();}
// if user is not admin or user or mod there must be something wrong so we kick them
else {header('Location: kick.php?error=6');exit();}

You're not checking whether $_POST[..] exists at all or whether any data has been submitted via POST, this may cause Undefined index notices.
You're not supposed to use empty on variables that you're explicitly declaring. empty is shorthand for !isset($var) || !$var. Since your variables are guaranteed to be set (you define them in the previous two lines), no need to suppress error reporting here with empty.
You don't need to clean a username or password against any sort of injection. Injection issues must be mitigated by properly escaping data in the right context, not by restricting the allowed characters used. If you have a specific business rule for restricting allowed characters so be it, but injection prevention is not it.
You're MySQL-escaping the values before you're checking for their validity. You need to do that in the opposite order. Escaping is always the last thing you do. There's no real issue in this case, since you're not modifying the string after escaping and your validity check isn't influenced either; but you should still stick to that rule.
You're using an inadequate password hash. Use password_hash instead.
You could be using a switch instead of the if..else chain.
You really should be using prepared statements to avoid escaping altogether. I know you're trying to take it slow, but you (should) have to make that move eventually, so why not now?

You may want to read:

The Great Escapism (Or: What You Need To Know To Work With Text Within Text)
The Definitive Guide To PHP's isset And empty

In summary, I'd write it like this (using procedural style for your benefit):

if (!isset($_POST['form_user'], $_POST['form_password'])) {
    header('Location: index.php?error=1');
    exit;
}

session_start();
include 'database/connection.php';

$stmt = mysqli_prepare($database, 'SELECT * FROM `accounts` WHERE `member` = ?');
mysqli_stmt_bind_param($stmt, 's', $_POST['form_user']);
mysqli_stmt_execute($stmt);
$result = mysqli_stmt_get_result($stmt);
$user = mysqli_fetch_assoc($result);

if (!$user) {
    header('Location: index.php?error=4');
    exit;
}

if (!password_verify($_POST['form_password'], $user['password'])) {
    header('Location: index.php?error=5');
    exit;
}

$_SESSION['username'] = $user['member'];

switch ($user['privilege']) {
    case 'USER':
        header('Location: users/index.php');
        exit;
    case 'ADMIN':
        header('Location: admins/index.php');
        exit;
    case 'MOD':
        header('Location: mods/index.php');
        exit;
    default:
        header('Location: kick.php?error=6');
        exit;
}


CSS organization with BEM I've recently learned BEM & SASS, and I'd like to know if my BEM syntax is correct. I'm trying to create just the header of this page: https://blackrockdigital.github.io/startbootstrap-agency/

Are there any differences you would make to my class names? I get very confused about how to deal with elements when they are nested within other elements. I find that I often want to chain elements, like class="header__navbar__logo", but I've heard this is not good BEM.

Start Bootstrap

Services

Portfolio

About

Team

Contact

        Welcome To Our Studio!
        It's Nice To Meet You

```

Well, if you are using Bootstrap, you should not alter the classes that Framework is using, like navabar for example.
The basic with BEM is that you should chain classes in a way to create a meaningful hierarchy, like in a component, where you have an object that contains everything else. Like that you leverage SASS nesting to create a component-like structure.
For example:

  <navbar class="navigationBar navbar">

            Item-1
            Item-2
            Item-3

  </navbar>

        Welcome To Our Studio!
        It's Nice To Meet You

In this case the SASS will be:
.navigationBar {
    width:104px;

    &--logo { width:103px; }
    &--navigation {
      width:102px;

        &--item {
          width:101px;

            &--link {width:100px; }
        }
    }
}

.content {
  width:100px;

  &--heading--primary {
    width: 101px;

    &--main { width: 102px; }
    &--sub { width: 103px; }
  }
}

Check the SASS code with SassMeister (https://www.sassmeister.com) to see how it turns to CSS


Get list of the executables in the current directory This is what I've done;

import Control.Applicative
import Control.Monad
import qualified System.Directory as SD
main = do
    files <- SD.getDirectoryContents =<< SD.getCurrentDirectory
    exes <- filterM (liftA SD.executable <$> SD.getPermissions) files
    print exes

This works fine, but I can't help feeling that this code is somewhat redundant --- with filterM, liftA, <$> and =<< all present at once, things seem unnecessarily complicated.

How can I improve this piece of code?

For a quick reference:

SD.getCurrentDirectory :: IO FilePath
SD.getDirectoryContents :: FilePath -> IO [FilePath]
SD.executable :: Permissions -> Bool
SD.getPermissions :: FilePath -> IO Permissions

and I want to filter the contents of the current directory based on its being executable or not.

I wouldn't import System.Directory qualified, it doesn't export anything that could clash with standard names.

import Control.Applicative
import Control.Monad
import System.Directory

main :: IO ()
main = do
    files <- getDirectoryContents =<< getCurrentDirectory
    exes <- filterM (liftA executable <$> getPermissions) files
    print exes

You can toss out the Functor and Applicative stuff and just compose Monads.

import Control.Monad
import System.Directory

main :: IO ()
main = do
    files <- getDirectoryContents =<< getCurrentDirectory
    exes <- filterM (liftM executable . getPermissions) files
    print exes

You could also get rid of reverse bind and use the forward version, it's maybe more familiar?

import Control.Monad
import System.Directory

main :: IO ()
main = do
    files <- getCurrentDirectory >>= getDirectoryContents
    exes <- filterM (liftM executable . getPermissions) files
    print exes

I'd eliminate the explicit use of bind entirely though, then it becomes easier to turn this into a function that can work on directories other than the current one.

import Control.Monad
import System.Directory

main :: IO ()
main = do
    dir <- getCurrentDirectory
    printExecutables dir

printExecutables :: FilePath -> IO ()
printExecutables dir = do
    files <- getDirectoryContents dir
    exes <- filterM (liftM executable . getPermissions) files
    print exes


Thread safe singleton class to connect to Cassandra I am working with Cassandra and using the Datastax Java driver for it. Here is my singleton class where it makes a connection to Cassandra:

public class CassUtil {
  private static final Logger LOGGER = Logger.getInstance(CassUtil.class);

  private Session session;
  private Cluster cluster;

  private static class Holder {
    private static final CassUtil INSTANCE = new CassUtil();
  }

  public static CassUtil getInstance() {
    return Holder.INSTANCE;
  }

  private CassUtil() {
    List<String> servers = TestUtils.HOSTNAMES;
    String username =
        TestUtils.loadCredentialFile().getProperty(TestUtils.USERNAME);
    String password =
        TestUtils.loadCredentialFile().getProperty(TestUtils.PASSWORD);

    PoolingOptions opts = new PoolingOptions();
    opts.setCoreConnectionsPerHost(HostDistance.LOCAL,
        opts.getCoreConnectionsPerHost(HostDistance.LOCAL));

    Builder builder = Cluster.builder();
    cluster =
        builder
            .addContactPoints(servers.toArray(new String[servers.size()]))
            .withRetryPolicy(DowngradingConsistencyRetryPolicy.INSTANCE)
            .withPoolingOptions(opts)
            .withReconnectionPolicy(new ConstantReconnectionPolicy(100L))
            .withLoadBalancingPolicy(
                DCAwareRoundRobinPolicy
                    .builder()
                    .withLocalDc(
                        !TestUtils.isProduction() ? "DC2" : TestUtils.getCurrentLocation()
                            .get().name().toLowerCase()).withUsedHostsPerRemoteDc(3).build())
            .withCredentials(username, password).build();

    try {
      session = cluster.connect("testkeyspace");
      StringBuilder sb = new StringBuilder();
      Set<Host> allHosts = cluster.getMetadata().getAllHosts();
      for (Host host : allHosts) {
        sb.append("[");
        sb.append(host.getDatacenter());
        sb.append(host.getRack());
        sb.append(host.getAddress());
        sb.append("]");
      }
      LOGGER.logInfo("CONNECTED SUCCESSFULLY TO CASSANDRA CLUSTER: " + sb.toString());
    } catch (NoHostAvailableException ex) {
      LOGGER.logError("error= ", ExceptionUtils.getStackTrace(ex));
    } catch (Exception ex) {
      LOGGER.logError("error= " + ExceptionUtils.getStackTrace(ex));
    }
  }

  public void shutdown() {
    LOGGER.logInfo("Shutting down the whole cassandra cluster");
    if (null != session) {
      session.close();
    }
    if (null != cluster) {
      cluster.close();
    }
  }

  public Session getSession() {
    if (session == null) {
      throw new IllegalStateException("No connection initialized");
    }
    return session;
  }

  public Cluster getCluster() {
    return cluster;
  }
}

I wanted to see if my CassUtil constructor can be improved in any way. It looks like CassUtil has 40+ lines of code. I think it can be broken down to a few methods but I am not able to figure out how I should do it.

First thoughts

Before even commenting the code, I have to say that the intent of this class is for me totally obscure. When I encounter some xxxUtil class, I expect to find some static utility methods. However here, it isn't the case.

Second, I'm not sure that I understand why you make this class a singleton. Yes, everytime I see a singleton I challenge that because it is very likely that it is an anti-pattern. So I will ignore the singleton for the rest of my answer.

Purpose of this class

The next thing that bothers me is that I'm not able to find the purpose of this class. It has no behaviour (except maybe shutdown()) ! All you do is creating two objects (session and cluster) and returning them. So the internal state of this class has totally leaked out (via getSession() and getCluster()) and all the work done with these two object is done outside of CassUtil, which is a significant smell in OOP world.

More concretely

Leaving these "philosophical" aspects above, let's dig a bit in something more concrete, the code itself.

Immutability

Making things immutable always help to get a more maintainable code, therefore I would advice to make CassUtil final so that noone can break it by subclassing. I would also make session and cluster final so that it becomes clear for anyone reading your code that once you instanciated them, they won't change anymore.

public final class CassUtil {
    private final Session session;
    private final Cluster cluster;

PoolingOptions

Does these two lines really do something useful other than assigning an existing value to itself ?

PoolingOptions opts = new PoolingOptions();
opts.setCoreConnectionsPerHost(HostDistance.LOCAL,opts.getCoreConnectionsPerHost(HostDistance.LOCAL));

If not, you can just remove these lines and do

.withPoolingOptions(new PoolingOptions())

StringBuilder

At the end of the constructor, you iterate through all hosts and append some content in a StringBuilder just to add it in a log statement. Wouldn't it be enough to do something like that ? (or equivalent)

LOGGER.logInfo("CONNECTED SUCCESSFULLY TO CASSANDRA CLUSTER: " + cluster.getClusterName());

Thus removing the need for the whole block constructing the StringBuilder:

StringBuilder sb = new StringBuilder();
Set<Host> allHosts = cluster.getMetadata().getAllHosts();
for (Host host : allHosts) {
    sb.append("[");
    sb.append(host.getDatacenter());
    sb.append(host.getRack());
    sb.append(host.getAddress());
    sb.append("]");
}

Get rid of TestUtils

I notice that your code depends a lot on TestUtils, which is static. And static means that your class is really hard to test. Let me reiterate: static dependencies make your class really (if not impossible) hard to test.

TestUtils provides 4 things:

Hostnames
A username
A password
Indirectly, a localDc

You better take all of these as arguments in the constructor, so that you don't need to reference TestUtils directly.

public CassUtil(List<String> servers, String username, String password, String localDc) {
    ...
}

This line

!TestUtils.isProduction() ? "DC2" : TestUtils.getCurrentLocation().get().name().toLowerCase()

is really the biggest smell of all these four because you actually mixed some production and testing concepts in the same place. That's why I advised you to take directly the localDc in argument instead of a boolean isProduction.

Creating the session

I'm not sure it makes sense here to embed the session's creation inside a try/catch. I would just remove it and let the exception bubble up, if ever. If you think it's probable exceptions will be raised from this call, then this instruction doesn't belong to the constructor, but should rather be in the method(s) where session is used (or in a connect() method, which would be the counterpart of shutdown()). So instead of

try {
    session = cluster.connect("testkeyspace");
    LOGGER.logInfo("CONNECTED SUCCESSFULLY TO CASSANDRA CLUSTER: " + cluster.getClusterName());
} catch (NoHostAvailableException ex) {
    LOGGER.logError("error= ", ExceptionUtils.getStackTrace(ex));
} catch (Exception ex) {
    LOGGER.logError("error= " + ExceptionUtils.getStackTrace(ex));
}

Just do

session = cluster.connect("testkeyspace");
LOGGER.logInfo("CONNECTED SUCCESSFULLY TO CASSANDRA CLUSTER: " + cluster.getClusterName());

Something that raised a warning in my head is the hardcoded string "testkeyspace". I suspect that this name should be different if you try to create a session in production or in testing. If so, you also need to take this string in the constructor's argument.

Remove null checks

Since now session and cluster are final, it is guaranteed that they won't ever be null. shutdown() and getSession() thus become:

public void shutdown() {
    LOGGER.logInfo("Shutting down the whole cassandra cluster");
    session.close();
    cluster.close();
}

public Session getSession() {
    return session;
}

Usage

Now that the constructor has changed (taking now 5 arguments) you might ask how to construct instances of CassUtil ?

For testing purpose:

new CassUtil(TestUtils.HOSTNAME, TestUtils.USERNAME, TestUtils.PASSWORD, "DC2", "testkeyspace");

In production:

new CassUtil(/*some real server names*/, /*username of current user*/, /*password of current user*/, /*name of the production local datacenter*/, /*production keyspace*/);

Conclusion

This is the whole class with all of these remarks:

import java.util.List;

import com.datastax.driver.core.Cluster;
import com.datastax.driver.core.PoolingOptions;
import com.datastax.driver.core.Session;
import com.datastax.driver.core.policies.ConstantReconnectionPolicy;
import com.datastax.driver.core.policies.DCAwareRoundRobinPolicy;
import com.datastax.driver.core.policies.DowngradingConsistencyRetryPolicy;

public final class CassUtil {
    private static final Logger LOGGER = Logger.getInstance(CassUtil.class);
    private final Session session;
    private final Cluster cluster;

    public CassUtil(List<String> servers, String username, String password, String localDc, String keyspace) {
        cluster = Cluster.builder().addContactPoints(servers.toArray(new String[servers.size()]))
                                   .withRetryPolicy(DowngradingConsistencyRetryPolicy.INSTANCE)
                                   .withPoolingOptions(new PoolingOptions())
                                   .withReconnectionPolicy(new ConstantReconnectionPolicy(100L))
                                   .withLoadBalancingPolicy(DCAwareRoundRobinPolicy.builder().withLocalDc(localDc)
                                                                                .withUsedHostsPerRemoteDc(3)
                                                                                .build())
                                   .withCredentials(username, password)
                                   .build();
        session = cluster.connect(keyspace);
        LOGGER.logInfo("CONNECTED SUCCESSFULLY TO CASSANDRA CLUSTER: " + cluster.getClusterName());
    }

    public void shutdown() {
        LOGGER.logInfo("Shutting down the whole cassandra cluster");
        session.close();
        cluster.close();
    }

    public Session getSession() {
        return session;
    }

    public Cluster getCluster() {
        return cluster;
    }
}

Now your class looks more clean but also presents the terrible signs of anemia (as expressed in the first two chapters of this answer) ! Think about that class: "which services should this class offer ?". Unfortunately, I can't answer that for you. But I'm sure as soon as you find the answer you will be able to find it a better name and add it new methods that actually contains business logic.