Fetch data from multiple tables I have three primary tables:

Customer
Product
Order

Relations:

Many to many between product and order.

CREATE TABLE Customer (
    CustomerId int  NOT NULL AUTO_INCREMENT,
    CustomerName varchar(50)  NOT NULL,
    CustomerMail varchar(50)  NOT NULL,
    CONSTRAINT Customer_pk PRIMARY KEY (CustomerId)
);

-- Table `Order`
CREATE TABLE `Order` (
    OrderId int  NOT NULL AUTO_INCREMENT,
    OrderDate date  NOT NULL,
    CustomerId int  NOT NULL,
    CONSTRAINT Order_pk PRIMARY KEY (OrderId)
);

-- Table Product
CREATE TABLE Product (
    ProductId int  NOT NULL AUTO_INCREMENT,
    ProductName varchar(50)  NOT NULL,
    SKU varchar(50)  NOT NULL,
    Price decimal(8,2)  NOT NULL,
    CONSTRAINT Product_pk PRIMARY KEY (ProductId)
);

-- Table ProductOrder
CREATE TABLE ProductOrder (
    ProductOrderId int  NOT NULL AUTO_INCREMENT,
    OrderId int  NOT NULL,
    ProductId int  NOT NULL,
    Quantity int  NOT NULL,
    CONSTRAINT ProductOrder_pk PRIMARY KEY (ProductOrderId)
);

ALTER TABLE `Order` ADD CONSTRAINT Order_Customer FOREIGN KEY Order_Customer (CustomerId)
    REFERENCES Customer (CustomerId);
-- Reference:  ProductOrder_Order (table: ProductOrder)

ALTER TABLE ProductOrder ADD CONSTRAINT ProductOrder_Order FOREIGN KEY ProductOrder_Order (OrderId)
    REFERENCES `Order` (OrderId);
-- Reference:  ProductOrder_Product (table: ProductOrder)

ALTER TABLE ProductOrder ADD CONSTRAINT ProductOrder_Product FOREIGN KEY ProductOrder_Product (ProductId)
    REFERENCES Product (ProductId);

I need to write a query for:

    Create a query for getting the total quantity of products
 ordered and the total value for all orders made
 on the current date.

This is my solution:

-- query for current date
SELECT SUM(quantity) AS TotalQuantity, SUM(quantity * price) AS TotalSaleAmount
     FROM productorder, product, `order`
         WHERE productorder.ProductId = product.ProductId
            AND
                `order`.OrderId = productorder.OrderId
            AND
                `order`.OrderDate = CURDATE()

Can someone help me in optimizing it?

I am getting the output but I am not sure if this is correct. I saw one or two solution using joins So I am confused that whether the query I have written is correct or not

It is a good practice to use JOIN : Explicit vs implicit SQL joins.

Second you could use aliases so you do not have to type entire table name.

SELECT  SUM(po.quantity)         AS TotalQuantity
       ,SUM(po.quantity*p.price) AS TotalSaleAmount
FROM productorder po
JOIN product p
  ON po.ProductId = p.ProductId
JOIN `order` o
  ON o.OrderId = po.OrderId
WHERE o.OrderDate = CURDATE();

SqlFiddleDemo


Python image combiner reading writing I am writing a simple script that will combine two images at a pixel level, selecting every other pixel from one image, and the other pixels from a second image.

from PIL import Image
import struct

print("Photo 1: ", end="")
photo1 = Image.open(input())  # First image, 1920 by 1080
photo1 = photo1.convert("RGB")

print("Photo 2: ", end="")
photo2 = Image.open(input()) # Second image, 1920 x 1080
photo2 = photo2.convert("RGB")

width = photo1.size[0]  # define W and H
height = photo1.size[1]

print("Output file: ", end="")
output = input()
data = str.encode("")
for y in range(0, height):  # counts to 1079
    row = ""
    for x in range(0, width):# counts to 1920
        if ((y + x) % 2) == 0: # makes every other pixel and alternates
                               # on each line, making a checker board pattern
            RGB = photo1.getpixel((x, y))
            print("Even", y) # how far am I?
        else:
            RGB = photo2.getpixel((x, y))
            print("Odd", y) # how far aim I?
        R, G, B = RGB  # now you can use the RGB value
        data += struct.pack("B", R) + struct.pack("B", G) + struct.pack("B",B)
                 # store RGB values in byte format in variable "data"

new = Image.frombytes("RGB", (1920, 1080), data)# change value if not 1920 x 1080
new.save(output, "PNG") # make new image

Sorry if the copy/paste ing messes with python indentation.
This script take a good 10 minutes to run on my laptop

Intel Core i5
8 GB ram
Running linux (Fedora 30)
Is this just going to take 10 minutes to grab all the individual pixel values, or is there any thing that is making this code inefficient?

Also, I am using python3.7 and 3 at the moment, but I also can use 2.7 if needed.

These are the two photos I am using:

Tuple unpacking

width = photo1.size[0]  # define W and H
height = photo1.size[1]

can be

width, height = photo1.size

Use input properly

Don't print here:

print("Output file: ", end="")
output = input()

Instead,

output = input('Output file: ')

Range default start

Don't include the 0 in these calls:

for y in range(0, height):  # counts to 1079
    for x in range(0, width):# counts to 1920

Only call pack once

    data += struct.pack("B", R) + struct.pack("B", G) + struct.pack("B",B)

should become, I think,

data += struct.pack('BBB', R, G, B)


Radix sort (LSD) and counting sort I'm reading CLRS, and to practice, I rolled my version of radix sort and counting sort. I was looking at some reference implementations, particularly the LSD one from Rosetta Code, and mine performs significantly better (~10 times), especially on 64 bit inputs and if the maximum input range is known.

I think one place that could be improved is the creation of res inside counting sort. It'd be faster if I could somehow use reserve as right now I default initialize everything and then assign over them. However, I can't grow it using push_back since in the following loop, res is grown randomly, rather than linearly as required by push_back. I don't know of any data structure that can have a runtime set size with unintialized values that also gets optimized nicely in the copy line after the loop.

I experimented with variable length arrays such as T res[n] and it works up to the stack limit. However, limiting input size to the size of my stack is not acceptable. Anyway, this change did not have a noticeable impact on performance.

Suggestions for improvements would be appreciated.

Repository of algorithms

Counting sort

// counting sort, assumes each input is integral between 0 to k
// O(n) if k = O(n)
template <typename Iter, typename Op>
void cnt_sort(Iter begin, Iter end, size_t k, Op op) {
    vector<int> counts(k);   // init to 0
    for (auto i = begin; i != end; ++i) // count # elems == i
        ++counts[op(*i)];
    for (size_t i = 1; i < k; ++i)
        counts[i] += counts[i-1];   // turn into # elems <= i
    vector<typename Iter::value_type> res(distance(begin, end)); // doing useless initialization here
    for (auto j = end;;) {
        --j;
        res[--counts[op(*j)]] = *j;
        if (j == begin) break;
    }
    copy(res.begin(), res.end(), begin); // compiler optimizes this out
}

Radix sort

// radix sort, more practical than counting sort
// O(d(n + k)) running time where d is # digits, k is size of digit
class Digit_cmp {   // functor for comparing a "digit" (particular bits)
    const long long mask; // 0..63 bitfield to test against
    const size_t to_shift;
public:
    Digit_cmp(long long m, size_t ts) : mask(m), to_shift(ts) {}
    template <typename T>
    T operator()(T n) const {
        return (n & mask) >> to_shift; // mask then shift to unit digit ex. 0xfab20000 >> 16
    }
};

template <typename Iter>
void rdx_sort(Iter begin, Iter end, int bits) {
    // bits is # bits to consider up to if a max val is known ahead of time
    // most efficent when digits are base n, having lg(n) bits
    size_t r {static_cast<size_t>(log2(end - begin))};   // # bits in digit
    size_t k {static_cast<size_t>(pow(2, r))};           // range of digit
    size_t d {0};                   // current digit num
    for (long long mask = ~(~0 << r);; // ex. 0x000000ff for setting lower 8 bits on 32 bit num
        mask <<= r) {
        cnt_sort(begin, end, k, Digit_cmp(mask, r*d));
        ++d;
        if (mask & (1 << (bits-1))) break; // finished masking most significant digit
    }
}
template <typename Iter>   // range of input not known, just use max ex. 32 bits for ints
void rdx_sort(Iter begin, Iter end) {
    int bits {sizeof(typename Iter::value_type)*CHAR_BIT};
    rdx_sort(begin, end, bits);
}

I found a list a few things that you could improve, but nothing that would significantly change the way your sorts work:

At the end of the counting sort, you could use std::move instead of std::copy to move the elements to the original collection. For simple integers, it shouldn't change anything but it could make a significant difference if you try to sort big integers.
There is a more idiomatic way to compute sizeof(typename Iter::value_type)*CHAR_BIT in the standard library thanks to std::numeric_limits::digits, so you can actually rewrite rdx_sort as follows:

template <typename Iter>   // range of input not known, just use max ex. 32 bits for ints
void rdx_sort(Iter begin, Iter end) {
    int bits {std::numeric_limits<typename Iter::value_type>::digits};
    rdx_sort(begin, end, bits);
}

Any number library (like a big number library) is allowed to specialize std::numeric_limits and every decent numbers library does so, so I shouldn't make your code less portable. By the way, the returned value is not always int, so the best thing you could do is to use a type alias to make things clearer:

template <typename Iter>   // range of input not known, just use max ex. 32 bits for ints
void rdx_sort(Iter begin, Iter end) {
    using value_type = typename Iter::value_type
    value_type bits {std::numeric_limits<value_type>::digits};
    rdx_sort(begin, end, bits);
}

Be careful of wild assumptions:

const long long mask; // 0..63 bitfield to test against

long long is not guaranteed to be a 64-bit integer, even if it is more often the case than not. If you want to avoid problems, you should use std::int64_t from <cstdint>. This type only exists if the architecture actually has a 64-bit integer, bug I guess that you will have more problems anyway if your implementation does not.

const std::int64_t mask; // 0..63 bitfield to test against

It seems that you are using using namespace std;, which is something you should really avoid in a header-only library like yours. Using this directive will import every name from std:: into the global namespace for anyone using your library and you will for sure end with name clashes at some point. Consider std::-qualifying everything from the C and C++ standard libraries (except macros of course), even std::size_t: some implementations don't import the names from the C standard library into the global namespace.
Dropping random vowels from function names isn't going to help anymore. Consider using full names like counting_sort and radix_sort instead.


Ruby Select uniq in one statement I'm trying to implement some simple logic using rails. Code is working fine but its look uglier to me, could any one help me on refractor or implement in a efficient way.

Context:

Data hash assetType key which can be image/clip and Sizes key has array of hashes.

if asset type is image then I'm filtering Sizes array if each hash has TeeShirtSize or not.

Same way all Sizes array of hashes should have Description and be unique.

each statement runs only if Sizes array not empty.

def get_parsed_asset_data(data)
     data['Sizes'].select!{|size| size.key?('TeeShirtSize')} if data['AssetType'] == 'Image' && data['Sizes'].any?
     data['Sizes'].select!{|size| size.key?('Description')} if data['Sizes'].any?
     data['Sizes'].uniq!{|size| size['Description']} if data['Sizes'].any?
     data
  end

First of all, I'd always avoid side-effects. That is, don't modify an object that was passed in; return a new object instead. So no !-methods.

Second, I'd reduce the method to only handle the Sizes array itself. Don't call the method unless there's something to process. That means checking AssetType and data['Sizes'].any? elsewhere.

Third, the name of the method is non-descriptive, in my opinion. It doesn't get anything, since it modifies what you give it, and "parsed asset data" is so generic that it doesn't tell me what the method actually does. Obviously it's something to do with t-shirts and their sizes and descriptions, so that should probably feature in the name.

Then you're left with something like:

def get_unique_tshirt_sizes(sizes) # or some other name
  sizes
    .select { |size| size.key?('TeeShirtSize') && size.key?('Description') }
    .uniq { |size| size['Description'] }
end

This method operates only on the sizes array, and it returns a new array rather than destructively updating an existing one.

You might use it like so:

if data['AssetType'] == 'Image' && data['Sizes'].any?
  sizes = get_unique_tshirts_sizes(data['Sizes'])
end


Insertion sort in ruby The code is correct, passes all testcases, I am looking for input on cleaning it, readability etc

def  insertionSort(a)
    return if a.nil? || a.empty? || a.length == 1
    sort_index = 0
    for i in 1..a.length-1 do
        for j in 0..sort_index do
            next if a[i] > a[j]
            temp = a[i]
            sort_index.downto(j) do |n|
               a[n+1] = a[n]
            end
            a[j] = temp
        end
        sort_index +=1
    end
  a
end

Ruby tends to use two spaces for indents, rather than four.

Avoid for; it's very uncommon in Ruby and, while technically correct, doesn't make full use of Ruby's functionality. In the general case, you can replace

for a in b do

with

a.each do |b|

and the rest of the code is (probably) identical. For this case, however, since you're iterating over a range of integers, I'd suggest using upto:

1.upto(a.length-1) do |i|
  0.upto(sort_index) do |j|

Or, if you feel like you really should have for a in b or b.each do |a|, you can use 1...a.length to exclude the last element, instead of subtracting one.

Use more informative variable names -- i and j might make sense to you now, but it's rather difficult to understand if you don't already know how insertion sort works.

I don't see a difference between sort_index and i - 1, though I haven't had a chance to run it and test it yet.

You should return a if a.length == 1, not nothing, since sorting a 1-item array is still sorting. Because the sort won't do anything, you can just remove the check entirely from the first if and forget about it.

Sorts in Ruby should use <=> instead of < or > because <=> is the universal comparison operator. See the docs for Object#<=> for more information.


Check for balanced parentheses in JavaScript I just implemented the balanced parentheses problem in JavaScript. For those of you who are new to this problem, a statement of it would be:

  Give a string of parentheses (including braces and square brackets),
  check to see if they are opened and closed in order.  The last opened
  parens should be closed first before others. '[()]{}{()()}' is
  balanced.  However, '( (] ([)]' is not.

Here is my solution:

  Algorithm

  Take a string input, such as '{([])}'. (Let's call each of
  the characters a 'paren'.)

  We validate the input, check if it is a string and has at least one paren.
  We initialize a stack and iterate through the string input.
  If the character is a closing paren, we check to see if it is the matching, closing paren of the top stack element. Note: This checks
  for the order, as the latest open parens have to be closed first in a
  balanced string of parens.
  If the character is an open paren, we push it onto the stack.
  If step 3 fails, even once, return false.
  After the loop, if the stack is empty, return true. (If all open parens were closed in order, the stack will be empty.)

var parenthesesAreBalanced = function(str) {
    var stack = new Stack(), l = str.length,
        currentVal, position, parenType,
        parentheses = '[]{}()';

    /* If it is not a string, return false */
    if(typeof str != 'string') {
        return false;
    }

    var foundParen = str.split('').some(function(el) {
        return (parentheses.indexOf(el) > -1);
    });

    /* If there is not a single instance of a paren, return false */
    if(!foundParen) return false;

    for(var i = 0; i < l; i++) {
        currentVal = str[i];

        position = parentheses.indexOf(currentVal);

        if(position === -1) {
            continue;
        }

        var parenType = (position % 2) ? 'closed' : 'open';

        if(parenType == 'closed') { /* 2. */
            var el = stack.pop();
            if(parentheses.indexOf(el) !== position - 1) {
                return false;
            }
        } else { /* 3. */
            stack.push(currentVal);
        }
    }
    /* 5. */
    return !stack.length();
};

I would appreciate any feedback on the performance and correctness of this implementation.

It looks like the time complexity is \$O(N)\$. How can it be improved with respect to the multiple, \$k\$, in \$O(N)\$?
What are some edge cases I should be testing? I tested the empty string, non-string values, and non-paren characters.

The algorithm looks fine to me, but I did not run it, as I do not have any stack implementation at hand. I expect most implementations to throw an exception when trying to pop from an empty stack, so your code may exhibit a weakness here.

Using a string as your preferred data structure to store and iterate parens looks a bit dubious. It implies repeated calls to indexOf() over the same values, and does not explicit well enough the nature of your parens (you need to track positions). Other approaches could be favored, e.g. maps. For instance, you could map opening tokens with their closing tokens, and conversely.

From a design point of view, your function mixes two things: the balance check itself and the parentheses which it should use to operate. I believe it would be cleaner to decouple these, for instance passing a list of Parens as argument to the function, each Parens indicating an opening token and a closing token. The design of your function would evolve in a good way, and you'd gain additional benefits as well (Open Closed Principle).


Brainfuck with C# operator overloading I overloaded some C# operators to get a pseudo version of the Brainfuck symbols in the effort of understanding the flow of this language.

I got this version:

void Main()
{
    /*
    *   Write `Hello World!`
    *           using operators overloading for brainfuck symbols
    */
    var _ = new brainfuck();

    _ = + + + + +    + + + + +     _        ; // set cell #0 to 10

    while (_)
    {
        _ =     + + + + +    + +       (_ > 1)              ; // add 7 to #1
        _ =     + + + + +    + + + + + (_ > 1)              ; // add 10 to #2
        _ =     + + +                  (_ > 1)              ; // add 3 to #3
        _ =     +                      (_ > 1)              ; // add 1 to #4

        _ =     -                      (_ < 1 < 1 < 1 < 1)  ; // decrement 0
    }
    _ =     ~   + +                    (_ > 1)              ; // write 'H'
    _ =     ~   +                      (_ > 1)              ; // write 'e'
    _ =     ~   + + + + +    + +        _                   ; //write 'l'
    _ =     ~                           _                   ; //write 'l'
    _ =     ~   + + +                   _                   ; // write 'o'
    _ =     ~   + +                     (_ > 1)             ; // write ' '
    _ =     ~   + + + + +    + + + + +   + + + + +      (_ < 1 < 1)     ; // write 'W'
    _ =     ~                           (_ > 1)             ; // write 'o'
    _ =     ~   + + +                   _               ; // write 'r'
    _ =     ~   - - - - -    -          _               ; // write 'l'
    _ =     ~   - - - - -    - - -      _               ; // write 'd'
    _ =     ~   +                       (_ > 1)         ; // write '!'
    _ =     ~                           (_ > 1)         ; // write '\n'
}
public class brainfuck
{
    private List<int> cells;
    private int currentCell;
    public brainfuck()
    {
        cells = new List<int>{0};
    }

    public static brainfuck operator + (brainfuck bf)
    {
        bf.plus();
        return bf;
    }
    public static brainfuck operator - (brainfuck bf)
    {
        bf.minus();
        return bf;
    }
    public static brainfuck operator > (brainfuck bf, int pos = 1)
    {
        bf.right();
        return bf;
    }
    public static brainfuck operator < (brainfuck bf, int pos = 1)
    {
        bf.left();
        return bf;
    }

    public static brainfuck operator ~ (brainfuck bf)
    {
        bf.write();
        return bf;
    }
    public static brainfuck operator ! (brainfuck bf)
    {
        bf.read();
        return bf;
    }
    public static implicit operator bool (brainfuck bf)
    {
        return bf.has_data();
    }

    private int plus()
    {
        return ++cells[currentCell];
    }
    private int minus()
    {
        return --cells[currentCell];
    }
    private void right()
    {
        if (currentCell + 1 == cells.Count)
            cells.Add(0);
        currentCell++;
    }
    private void left()
    {
        if (currentCell > 0)
            currentCell--;
    }
    private int current()
    {
        return cells[currentCell];
    }
    private void write()
    {
        Console.Write((char)current());
    }
    private void read()
    {
        var input = Console.ReadLine();
        int value = Int32.TryParse(input, out value) ? value : 0;
        current(value);
    }
    private bool has_data()
    {
        return current() > 0;
    }
    private void current(int value)
    {
        cells[currentCell] = value;
    }

    public override string ToString()
    {
        return String.Join(",",cells);
    }
}

I had to cheat with the [] command pair using the while statement and the flow is very odd from the point of Brainfuck reader, since the <,> commands are before the +,- in the original Hello World! example, and with C# program are in the opposite way.

Do you think I can get a closer version to Brainfuck than the current one?

You could get slightly closer with the [] command pair.

It's actually quite trivial:

Add an indexer to your brainfuck class (public void this[Action bf]) and put your while loop in there.

The indexer would look like:

public brainfuck this[Action action]
{
    get
    {
        while (this)
        {
            action.Invoke();
        }

        return this;
    }
}

You would call it as:

_ = _[() => {
    _ =     + + + + +    + +       (_ > 1)              ; // add 7 to #1
    _ =     + + + + +    + + + + + (_ > 1)              ; // add 10 to #2
    _ =     + + +                  (_ > 1)              ; // add 3 to #3
    _ =     +                      (_ > 1)              ; // add 1 to #4

    _ =     -                      (_ < 1 < 1 < 1 < 1)  ; // decrement 0
}];

Is it a pain in the rear to use? Yes. Is it in the Brainfuck spirit? Yes.

It's still not true brainfuck syntax, but you won't get there with C#.

After that, you have a real mess here that needs cleaned.

First: capitalization. In C# we PascalCase public members and types (and all methods), and camelCase private and local members and types.

public class Brainfuck
private bool HasData()

Since "Brainfuck" is one word, the "F" in "fuck" is not capitalized.

The optional argument defaults in operator > and operator < are not at all going to work, remove them.

public static brainfuck operator > (brainfuck bf, int pos)
{
    bf.right();
    return bf;
}
public static brainfuck operator < (brainfuck bf, int pos)
{
    bf.left();
    return bf;
}

Always use braces, even when you don't need them.

private void Right()
{
    if (currentCell + 1 == cells.Count)
    {
        cells.Add(0);
    }

    currentCell++;
}

Use int instead of Int32 everywhere.

int value = int.TryParse(input, out value) ? value : 0;

Once we apply all these changes, fix some spacing, we end up with a not-so-bad looking class:

public class Brainfuck
{
    private List<int> cells;
    private int currentCell;

    public Brainfuck()
    {
        cells = new List<int>{0};
    }

    public Brainfuck this[Action action]
    {
        get
        {
            while (this)
            {
                action.Invoke();
            }

            return this;
        }
    }

    public static Brainfuck operator + (Brainfuck bf)
    {
        bf.Plus();
        return bf;
    }

    public static Brainfuck operator - (Brainfuck bf)
    {
        bf.Minus();
        return bf;
    }

    public static Brainfuck operator > (Brainfuck bf, int pos)
    {
        bf.Right();
        return bf;
    }

    public static Brainfuck operator < (Brainfuck bf, int pos)
    {
        bf.Left();
        return bf;
    }

    public static Brainfuck operator ~ (Brainfuck bf)
    {
        bf.Write();
        return bf;
    }

    public static Brainfuck operator ! (Brainfuck bf)
    {
        bf.Read();
        return bf;
    }

    public static implicit operator bool (Brainfuck bf)
    {
        return bf.HasData();
    }

    private int Plus()
    {
        return ++cells[currentCell];
    }

    private int Minus()
    {
        return --cells[currentCell];
    }

    private void Right()
    {
        if (currentCell + 1 == cells.Count)
        {
            cells.Add(0);
        }

        currentCell++;
    }

    private void Left()
    {
        if (currentCell > 0)
        {
            currentCell--;
        }
    }

    private int Current()
    {
        return cells[currentCell];
    }

    private void Write()
    {
        Console.Write((char)Current());
    }

    private void Read()
    {
        var input = Console.ReadLine();
        int value = int.TryParse(input, out value) ? value : 0;
        Current(value);
    }

    private bool HasData()
    {
        return Current() > 0;
    }

    private void Current(int value)
    {
        cells[currentCell] = value;
    }

    public override string ToString()
    {
        return String.Join(",",cells);
    }
}

And our code for the "Hello World" looks like:

void Main()
{
    /*
    *   Write `Hello World!`
    *           using operators overloading for brainfuck symbols
    */
    var _ = new Brainfuck();

    _ = + + + + +    + + + + +     _        ; // set cell #0 to 10

    _ = _[() => {
        _ =     + + + + +    + +       (_ > 1)              ; // add 7 to #1
        _ =     + + + + +    + + + + + (_ > 1)              ; // add 10 to #2
        _ =     + + +                  (_ > 1)              ; // add 3 to #3
        _ =     +                      (_ > 1)              ; // add 1 to #4

        _ =     -                      (_ < 1 < 1 < 1 < 1)  ; // decrement 0
    }];
    _ =     ~   + +                    (_ > 1)              ; // write 'H'
    _ =     ~   +                      (_ > 1)              ; // write 'e'
    _ =     ~   + + + + +    + +        _                   ; //write 'l'
    _ =     ~                           _                   ; //write 'l'
    _ =     ~   + + +                   _                   ; // write 'o'
    _ =     ~   + +                     (_ > 1)             ; // write ' '
    _ =     ~   + + + + +    + + + + +   + + + + +      (_ < 1 < 1)     ; // write 'W'
    _ =     ~                           (_ > 1)             ; // write 'o'
    _ =     ~   + + +                   _               ; // write 'r'
    _ =     ~   - - - - -    -          _               ; // write 'l'
    _ =     ~   - - - - -    - - -      _               ; // write 'd'
    _ =     ~   +                       (_ > 1)         ; // write '!'
    _ =     ~                           (_ > 1)         ; // write '\n'
}

Overall, not too bad. I wouldn't have done it, but that's because I wrote a Brainfuck to C# transpiler. Good work. :)


Semantic HTML5 structure and Google penalty I'm in need for clarity. I'm at an annoying step in my development of a website, and I have read a lot about semantic structure etc, but I'm a bit nervous for my current structure, so I would like to ask your expertise to see, if I might get problems with search engine bots with this setup.

    Skip menuen

                <svg id="header-logo" class="header-logo" xmlns="http://www.w3.org/2000/svg" role="img" aria-labelledby="title desc" viewBox="0 0 158 24.7">
                    <title id="title" itemprop="headline">Brand Name</title>
                    <desc id="desc" itemprop="description">- brand description</desc>

                </svg>

            <button id="hamburger-btn">

            </button>

page

page

page

page

page

page

                    <button id="siteLogin" class="nav-btn">
                        Admin login
                        <svg class="nav-admin-icon nav-icons">
                            <use xlink:href="img/generel/svg-system.svg#admin-icon"></use>
                        </svg>
                    </button>

                <button id="siteSearch" class="nav-btn">
                    Søg
                    <svg class="nav-search-icon nav-icons">
                        <use xlink:href="img/generel/svg-system.svg#search-icon"></use>
                    </svg>
                </button>

Main content header

In the outline it will look like this:

Brand name - brand description

untitled nav
Main content header

Semantically, a heading should contain text, not an image.

Also, somewhat more important from an SEO perspective, the
 element should relate to the specific page. Unless this site is a single page, having the same
 on every page would be a very bad idea.

Generally, ranking for the company name is a given (unless the company is called 'Cheap Viagra Cialis Online ltd"...).

I would remove the heading tag from the page header:

            <svg id="header-logo" class="header-logo" xmlns="http://www.w3.org/2000/svg" role="img" aria-labelledby="title desc" viewBox="0 0 158 24.7">
                <title id="title" itemprop="headline">Brand Name</title>
                <desc id="desc" itemprop="description">- brand description</desc>

            </svg>

And swap the
 in the content section for an
:

Main content header


why does this programs speed not increase? This is the program I was given, it searches a large array for a max value:

import java.util.Date;
import java.util.Random;

class FindMax {
    private static final int N = 256 * 1024 * 1024;

    public static void main(String args[]) {
        assert(N > 0);

        int array[] = new int [N];

        assert(array != null);

        Random random = new Random(new Date().getTime());

        for (int i = 0; i < N; i++) {
            array[i] = random.nextInt();
        }

        Date start = new Date();

        int max = array[0];

        for (int i = 1; i < N; i++) {
            if (array[i] > max) {
                max = array[i];
            }
        }

        Date end = new Date();

        System.out.println("max: " + max);
        System.out.println("time in msec: " + (end.getTime() - start.getTime()));
    }
}

I am to increase the speed by having multiple threads each search part of the array. This is what i did:

import java.util.Date;
import java.util.Random;

class FindMax extends Thread{
    private static final int N = 256 * 1024 * 1024;

    static int array[] = new int [N];
    static int max = array[0];
    int start, end;

    public void run(){

        for (int i = this.start; i < this.end; i++) {
            if (array[i] > max) {
                max = array[i];
            }
        }

    }
    public FindMax(int q, int u){
        this.start=q;
        this.end=u;

    }

    public static void main(String args[]) {
        assert(N > 0);
        int ts = Integer.parseInt(args[0]);

        assert(array != null);

        Random random = new Random(new Date().getTime());

        for (int i = 0; i < N; i++) {
            array[i] = random.nextInt();
        }

        Date start = new Date();

        Thread t[] = new Thread[ts];
        int eacht = (256 * 1024 * 1024)/ts;

        for( int p=0; p<ts;p++){
            t[p] = new FindMax(0+p*eacht, (eacht-1)+p*eacht);
            t[p].start();
        }

        Date end = new Date();

        System.out.println("max: " + max);
        System.out.println("time in msec: " + (end.getTime() - start.getTime()));
    }
}

The problem is that is has the opposite effect. The more threads (taken from command line argument) the LONGER the program takes to run. Why is this doing the exact opposite of increasing the programs speed with increased threads created?

Contrary to your belief, running a program with more threads than the number of processors (Runtime.getRuntime().availableProcessors()) will not increase the performance of your program, in fact it will decrease performance. The decrease in performance occurs because your processor has to jump from thread to thread. That is not a time-free operation.

The exception to this rule being if you have a lot of blocking I/O operations in your threads, in that case, more threads than the number of processors may be appropriate for your use.

I suggest you use an ExecutorService which runs with the correct number of threads (possibly obtained through one of the factory methods in Executors) and submit tasks for it to complete.


C++ interview problem: finding an Island in a 2D grid I was asked this question from an online coding interview and I have provided my solution that passes all of the test cases. I wanted to see if someone can review my code.

You can find my solution below using BFS. Here's also the link to the question that was asked at Pramp.

  Island Count

  Given a 2D array binaryMatrix of 0s and 1s, implement a function getNumberOfIslands that returns the number of islands of 1s in binaryMatrix.

  An island is defined as a group of adjacent values that are all 1s. A cell in binaryMatrix is considered adjacent to another cell if they are next to each either on the same row or column. Note that two values of 1 are not part of the same island if they’re sharing only a mutual “corner” (i.e. they are diagonally neighbors).

  Explain and code the most efficient solution possible and analyze its time and space complexities.

input:  binaryMatrix = [ [0,    1,    0,    1,    0],
                         [0,    0,    1,    1,    1],
                         [1,    0,    0,    1,    0],
                         [0,    1,    1,    0,    0],
                         [1,    0,    1,    0,    1] ]

output: 6 # since this is the number of islands in binaryMatrix.
          # See all 6 islands color-coded below.

#include <iostream>
#include <vector>
#include <queue>
#include <unordered_set>

#define AND &&
using namespace std;

int getNumberOfIslands( const vector<vector<int>>& binaryMatrix )
{
  int ans=0;
  int n = binaryMatrix.size();
  if (!n) return 0;
  int m = binaryMatrix[0].size();
  unordered_set<int> *used = new unordered_set<int>[n];
  for (int i=0; i<n; i++) used[i].clear();
 //Implementation through Breadth First Search
 // starting origin for my x and y coordinates
  int dx[] = {-1, 1, 0, 0};
  int dy[] = {0, 0, -1, 1};
  queue< pair<int, int> > q;

  for (int i=0; i<n; i++)
    for (int j=0; j<m; j++){

      if (!used[i].count(j) AND binaryMatrix[i][j]==1){

        ans++;

        q.push(make_pair(i, j));
        used[i].insert(j);
        while (!q.empty()){
          pair<int, int> pos = q.front();
          q.pop();

          for (int k=0; k<4; k++){
            int nx = pos.first+dx[k];
            int ny = pos.second+dy[k];
            if (nx<0 || nx>=n) continue;
            if (ny<0 || ny>=m) continue;
            if (used[nx].count(ny)) continue;
            if (binaryMatrix[nx][ny]!=1) continue;

            used[nx].insert(ny);
            q.push(make_pair(nx, ny));

          }

        }

      }

    }

  return ans;

}

int main() {
  return 0;
}

My code passes all of the test cases:

Test Case #1
Input: [[0]],Expected: 0,Actual: 0
Test Case #2
Input: [[1]],Expected: 1,Actual: 1
Test Case #3
Input: [[1,0,1,0]],Expected: 2,Actual: 2
Test Case #4
Input: [[1,0,1,0],[0,1,1,1],[0,0,1,0]],Expected: 2,Actual: 2
Test Case #5
Input: [[1,0,1,0],[0,1,1,1],[0,0,1,0],[1,1,0,0],[0,1,0,1]],Expected: 4,Actual: 4
Test Case #6
Input: [[0,1,0,1,0],[0,0,1,1,1],[1,0,0,1,0],[0,1,1,0,0],[1,0,1,0,1]],Expected: 6,Actual: 6
Test Case #7
Input: [[1,1,1,1,1],[1,1,1,1,1],[1,1,1,1,1],[1,1,1,1,1],[1,1,1,1,1]],Expected: 1,Actual: 1

Don't use using namespace std

It's considered bad practice.

Don't define AND

There is already and as an alternative to &&.

Use std::vector instead of new[]

You have a memory leak at the moment, since you forgot to delete[] the allocated memory. std::vector prevents this kind of error.

Use appropriate data structures

You use an array of unordered_set<int>s where a std::unordered_set< std::pair<int,int> > would be sufficient. Also, clear() isn't necessary. Unless your std::unordered_set implementation is broken, a std::unordered_set starts out empty.

Consider const for variables that shouldn't change

dx, dy, n and m should never change. const makes sure that you don't change them by accident.

Always use braces around blocks

No braces after control structures have led to several bugs that were hard to catch. Always use them. That way you cannot accidentally forget to place them when necessary.


Binary search on cstrings This implements binary search on an array of cstrings. One thing I'm wondering about is it faster or slower to pass a variable as const i.e. len. Also strcmp() gets called a lot and since I can't find any specific details on how it works, is it worthwhile to make my own function? For example does it immediately return a value if the first character of each arguments are different?

/*
*IN: scope: array of cstrings to be searched
*IN: len: length of array
*IN: find: cstring to be found
*OUT: true if find is in scope, otherwise false
*/
bool binSrch(char** scope, const int len, char* find)
{
   int c, first, last, middle;

   first = 0;
   last = len - 1;
   middle = (first+last)/2;

   while( first <= last )
   {
      if (strcmp(scope[middle], find) < 0 )
         first = middle + 1;
      else if (strcmp(scope[middle], find) == 0)
      {
         return true;
      }
      else
         last = middle - 1;

      middle = (first + last)/2;
   }
   if ( first > last )
      return false;
}

EDIT: is there a way to intelligently choose the pivot point? For example if your looking in a phone book for a name starting with Z you wouldn't start at the middle you'd start closer to the back.

You can assume that the implementation of strcmp() is not stupid.  A lot of code out there relies on strcmp(), so rest assured that a lot of work has gone into optimizing it.  Nevertheless, you don't want to call it twice per iteration through your loop.

Interestingly, len is the only parameter that doesn't need to be declared const, since it is passed by value.  In contrast, scope and find are passed by reference, so a promise that binSrch() won't alter them would be helpful.

At the end, you don't need to test for first > last before returning false.  The only way you can get past the while loop is with first > last.

You have an unused variable c.  Avoid such mistakes by getting into the habit of assigning right away:

int first = 0,
    last = len - 1,
    middle = (first + last) / 2;

I think the braces on your else if look weird.  Either remove them, or put them everywhere consistently.  (I prefer the latter.)


Moving character in given direction C++ SFML I have written simple input manager which is used for me to detect key press, and fire assigned function. For example I can do something like that:
auto own_pointer = std::make_shared<Player>(*this);
inputControl.addKeyToCheck( sf::Keyboard::H, std::function<void( Player& )>( &Player::sayHello ),own_pointer );

and in my loop manager, I check input managers, and if H is pressed, then function sayHello will be fired.
I have written moving my player with this input manager.
auto own_pointer = std::make_shared<Player>(*this);
inputControl.addKeyToCheck( sf::Keyboard::A, std::function<void( Player& )>( &Player::moveLeft ),own_pointer );
inputControl.addKeyToCheck( sf::Keyboard::D, std::function<void( Player& )>( &Player::moveRight ), own_pointer );
inputControl.addKeyToCheck( sf::Keyboard::S, std::function<void( Player& )>( &Player::moveDown ), own_pointer );
inputControl.addKeyToCheck( sf::Keyboard::W, std::function<void( Player& )>( &Player::moveTop ), own_pointer );

where for example moveTop is just
void Player::moveTop()
{
    this->getComponent<Velocity>()->y -= speed * mv::constants::mob::DEFAULT_SPEED;
}

Generally I am so proud of my InputManager, it works everywhere else but there is really bad to use 4 different function to control moving. Have somebody any advice how to deal with that style problem?
Input manager
    /**
    * @brief Class which manage keys' input
    */
    template <class T>
    class InputManager
    {
        /* ===Objects=== */
    public:
    protected:
    private:
        //when you press KEY you execute FunctionWrapper
        std::map < sf::Keyboard::Key, FunctionPointerWrapper_t<T>> keyData;
        /* ===Methods=== */
    public:
        /**
        * @brief Checks if keys have been clicked
        */
        void update();

        /**
        * @bried adds key to database
        */
        bool addKeyToCheck( sf::Keyboard::Key key, std::function<void( T& )> function, std::shared_ptr<T> object );

        /**
        * @brief remove key from database
        */
        bool eraseKey( sf::Keyboard::Key key );
    protected:
    private:
    };

    template <class T>
    void InputManager<T>::update()
    {
        for ( auto&var : keyData )
            if ( sf::Keyboard::isKeyPressed( var.first ) )
                var.second.function( *var.second.object );
    }

    template <class T>
    bool InputManager<T>::addKeyToCheck( sf::Keyboard::Key key, std::function<void( T& )> function, std::shared_ptr<T> object )
    {
        keyData.emplace( key, FunctionPointerWrapper_t<T>( function, object ) );

        return true;
    }

    template <class T>
    bool InputManager<T>::eraseKey( sf::Keyboard::Key key )
    {
        keyData.erase( key );
        return false;
    }

FunctionPointerWrapper_t seems unnecessary; we can store a std::function<void()> in the map directly. To create this function, we can either use std::bind:

auto player = std::make_shared<Player>(*this);
inputControl.addKeyToCheck( sf::Keyboard::H, std::bind(&Player::sayHello, player) );

or a lambda function:

auto player = std::make_shared<Player>(*this);
inputControl.addKeyToCheck( sf::Keyboard::H, [=] () { player->sayHello(); } );

Note that both of these examples internally create a copy of the shared_ptr.

For cutting down on the number of named functions in the player class, we can route the functionality to a single move function, something like:

void Player::move(Vector2f const& speed) { getComponent<Velocity>() += speed * mv::constants::mob::DEFAULT_SPEED; }

We can then use std::bind, or lambda functions as above:

inputControl.addKeyToCheck( sf::Keyboard::W, std::bind(&Player::move, player, Vector2f(0.f, -1.f)) );
inputControl.addKeyToCheck( sf::Keyboard::W, [=] () { player->move(Vector2f(0.f, -1.f)); });

Where Vector2f(0.f, -1.f) can be replaced by whatever the actual desired type / value is. (We could also do the multiplication outside of the move function, if it's not common between the functions).

We still need to define 4 functions, but there's less overhead in the Player class.

To simplify it further, we'd probably want to define some sort of axis mapping, so we could do something like:

inputControl.addAxis(Axis(sf::Keyboard::W, sf::Keyboard::S, maxSpeed, minSpeed), [=] (float axisValue) { player->moveVertically(axisValue); });

(and perhaps even allow a 2d-axis to be defined mapping 4 keys to one function call) but that might be a fair amount of extra work.

General comments:

InputManager::addKeyToCheck and InputManager::eraseKey should probably not return a fixed true / false without checking if they actually succeeded.


Searching for nodes in a huge tree This is supposed to search for a node in a huge Treeview. The node could be found in many places. In other words there may be many nodes and the resulting tree should be expanded depending on where nodes are found.

I was asked to avoid recursivity in this method. The only solution I was able to get is using LINQ queries but I cannot since we are under 2.0 framework.

Can I avoid recursivity in this method?

  private void CheckFoundTreeNode(TreeNode startingTreeNode, List<Item> parentItems, ItemNode foundItemNode)
    {
        foreach (TreeNode childTreeNode in startingTreeNode.Nodes)
        {
            ItemNode itemNode = TreeBuilder.GetItemNode(childTreeNode);
            Item item = itemNode.Item;

            bool isParent;
            if (item.Ancestor == null)
                isParent = parentItems.Contains(item);
            else
            {
                Item ancestor = ((Grammar)item).DirectAncestor;
                isParent = parentItems.Contains(ancestor);
            }

            if (isParent)
            {
                childTreeNode.Expand();
                CheckFoundTreeNode(childTreeNode, parentItems, foundItemNode);
            }

            else if (itemNode == foundItemNode)
            {
                startingTreeNode.Expand();
                m_ListOfFoundNodes.Add(childTreeNode);
            }
        }
    }

The simplest way to avoid the recursive traversal is to have two lists: nodes you want to visit and nodes you have visited. Your loop then goes like this:

take a node from the 'to visit' list, or end if empty
check the node
add any connected nodes that aren't in the 'visited' list to the 'to visit' list


Configuring fields to be extracted by a data processor I solved this problem using closures with Perl and I wonder if/how I could/should have done it differently?

Background:

User creates config file describing properties of some source data file that would be processed:

Type of file delimited (and the delim) or fixed width
How many fields to expect
Which fields are to be pulled out

Examples on the last (if delimited):

Fields: FIRST_NAME 2
Fields: LAST_NAME 3
Fields: ADDRESS 4

or (fixed width):

Fields: FIRST_NAME 58,30
Fields: LAST_NAME  88,30
Fields: ADDRESS    118,50

My Perl reads the config, and prepares ahead the action to take once I start reading the source data file.

Internally, I'm creating preparing my master hash (per the config), which I'll loop on for each record when reading the source.

Now to handle the said examples, I have this code, if the source file would be delimited:

sub ret_col_sub
{
    my ($col_name, $col_pos) = @_

    warn qq($col_name col_pos='$col_pos' is not a number) if $col_pos =~ /\D/;

    $col_pos--;

    # Return closure to caller, which will be called later when we're reading the source data file
    return sub
    {
        my $rec = shift;

        return $rec->[$col_pos];
    };
}

or fixed width:

sub ret_substr_sub
{
    my ($col_name, $substr_params) = @_;

    my ($offset, $len) = $substr_params =~ /(\d+)\s*,\s*(\d+)/;

    # The offset might be ZERO, check for length ... but $len must always be P O S I T I V E !
    warn qq($col_name substr_params='$substr_params' is missing the offset and length needed by substr) if not length $offset or not $len;

    $offset--;
    $len--;

    # Return closure to caller, which will be called later when we're reading the source data file
    return sub
    {
        my $rec = shift;

        return substr $rec, $offset, $len;
    };
}

Is there a better or another way of doing this, or any other suggestions?

This seems like a desperate attempt to find a use for closures, and I believe your problem would be solved much more simply by making $rec an object, perhaps even a tied object that lets you overload indexing

Some observations:

It makes the API much less useful when the calling code has to know which form of data it is dealing with anyway. ret_col_sub returns a closure that expects a simple string for its parameter, while ret_substr_sub returns a closure that requires an array reference. All of that should be hidden from the caller
I can see that ret_col_sub won't compile
Checking that a string doesn't match \D isn't sufficient to ensure that it is numeric, as an empty string will pass that test
It is correct to rebase the substring offset, but a length is a length and that should be left untouched
The postfixed ++ and -- operators have become popular because of the name of the C++ language, but unless you are using the value returned by tge operator you should be using tge prefixed version. It is wrong to rely on the compiler to optimise out the storage of the unused original value
As has been said, your style contravenes common standards, in particular those laid out in perlstyle which are the most universal


Filling Array with Alphanumeric values I've done a function that does the following :

It takes an argument length that it will be the length of the result array.
The Array should start with alphabet values first : ['A', 'B', 'C', ...]
If the length of the Array is greater than 26 (The number of letters) it should look like :  ['A', 'B', 'C', ..., 'A1', 'B1', 'C1'], if greater than 52 : ['A', 'B', 'C', ..., 'A1', 'B1', 'C1', ..., 'A2', 'B2', 'C2'] and so on.

The following function i've done uses some new Array features :

let getNums = length =>{

  if(length <= 26) return Array.from({length}, (el, i)=> String.fromCharCode(i + 65));

  let nums = Math.floor( length / 26 );
      nums = nums == 0 ? 0 : nums - 1;
  let rest = length - (26 * (nums + 1) );

  return [...Array.from({length : 26}, (el, i)=> String.fromCharCode(i + 65)),
          ...Array.from({length : nums}, (el, i)=> i + 1)
            .reduce( (a, b)=> a.concat(Array.from({length : 26},
                     (el, i)=> String.fromCharCode( i + 65 ) + b ) ),
            []),
         ...Array.from({length : rest}, (el, i)=> String.fromCharCode(i + 65) + (nums + 1) )
 ];

}

console.log(getNums(53));

This works, but I'm worried about the efficiency of this function because several Arrays are created and concatenated. So, this would be better if done with loops and some if...else statements?

Unless you confirm the speed is a problem in the code profiler, there's no need to optimize it.

And if you want to make js code run fast, just avoid using one of the most expensive operations: invocation of a callback/function (you had 6 of them), maximize the workload inside the engine by using existing native functions and minimizing amount of time spent in interpreted code.

In this case switching to string manipulation makes the code 2.5 times faster as measured here on 10k repetitions.

let getNums = length => {
    var letters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';
    var result = letters.substr(0, length).split('');
    for (var i = 1, numberedCount = Math.floor((length - 1)/26); i <= numberedCount; i++) {
        length -= letters.length;
        result.push.apply(result,
            letters.substr(0, length).replace(/./g, '$&' + i + ' ').trim().split(' '));
    }
    return result;
}


Monty Hall Simulation I wrote this code as my first self-directed effort and would appreciate any input on things that I've done seriously wrong. I worry that perhaps I'm over-relying on if-statements, for example, but am not sure how to change the pieces of code to use them less.

import random

def playchosengame(chosengame):
    if chosengame == 1:
        doorchoice = int(raw_input("How many doors would you like to play with? > "))
        onegameatatime(doorchoice)
    elif chosengame == 2:
        numberofgames = int(raw_input("How many games would you like to play? > "))
        strategy = int(raw_input("Would you like to always change your answer (1) or keep it the same (2)? > "))
        doorchoice = int(raw_input("How many doors would you like to play with? > "))
        onestrategy(numberofgames,strategy,doorchoice)
    else:
        print "What did you say?"

def onestrategy(chosennumberofgames,chosenstrategy,doorchoice):
    # for playing x number of Monty Hall games with one type of strategy
    wincount = 0
    i = 1

    for i in range(1,chosennumberofgames + 1):
        possibleanswers = range(1,doorchoice + 1)
        correctanswer = random.randint(1,doorchoice)
        incorrectpossibilities = possibleanswers
        incorrectpossibilities.remove(correctanswer)

        youranswer = random.randint(1,doorchoice)

        if youranswer == correctanswer:
            otherremaining = random.choice(incorrectpossibilities)
        else:
            otherremaining = youranswer

        incorrectpossibilities.remove(otherremaining)

        print "The correct answer is NOT one of these: %r" % incorrectpossibilities

        if youranswer == correctanswer:
            print "Which means it is either your answer: %r, or the only other remaining option, %r." % (correctanswer, otherremaining)
            if chosenstrategy == 1:
                finalanswer = otherremaining
            else:
                finalanswer = correctanswer
        else:
            print "Which means it is either your answer: %r, or the only other remaining option, %r." % (youranswer, correctanswer)
            if chosenstrategy == 1:
                finalanswer = correctanswer
            else:
                finalanswer = youranswer

        print "You chose %r" % finalanswer

        if finalanswer == correctanswer:
            wincount += 1
            print "You win!"
        else:
            print "You lose!"

        i += 1

    print "You won %r out of %r games. Congrats!" % (wincount, i - 1)

def onegameatatime(doorchoice):
    # for playing one Monty Hall game at a time.
    playagain = 1

    while playagain == 1:
        possibleanswers = range(1,doorchoice + 1)
        correctanswer = random.randint(1,doorchoice)
        incorrectpossibilities = possibleanswers
        incorrectpossibilities.remove(correctanswer)

        youranswer = int(raw_input("Pick a door number from 1 - %r > " % doorchoice))

        if youranswer == correctanswer:
            otherremaining = random.choice(incorrectpossibilities)
        else:
            otherremaining = youranswer

        incorrectpossibilities.remove(otherremaining)

        print "The correct answer is NOT one of these: %r" % incorrectpossibilities

        if youranswer == correctanswer:
            print "Which means it is either your answer: %r, or the only other remaining option, %r." % (correctanswer, otherremaining)
        else:
            print "Which means it is either your answer: %r, or the only other remaining option, %r." % (youranswer, correctanswer)

        finalanswer = int(raw_input("Which do you choose? > "))

        if finalanswer == correctanswer:
            print "You win!"
        else:
            print "You lose!"

        playagain = int(raw_input("Play again? (1 = yes, 0 = no) > "))

    print "Thanks for playing!"

gamechoice = int(raw_input("Play one game at a time (1) or play your chosen number of games with one strategy (2)? > "))
playchosengame(gamechoice)

Welcome to Code Review and coding in general:

Code according to PEP8 – In your case this means using snake_case for variable and function names, add spaces after commas and between operators. Your code does look rather nice in general, some minor nitpicks here and there, but read through the guidelines and try to adhere to them.
Feature: incorrect_possibilities = possible_answers invalidates possible_answers – When you later on do incorrect_possiblities.remove(correct_answer) this also changes possible_answers. To get a copy you need either to use a better variant of copy, or simply incorrect_possibilities = possible_answers[:]. The latter one uses slicing to denote the entire array, and makes a copy of the sliced area.
Use ternary related to chosen_strategy - Based on the chosen strategy you choose either of two choices, this can be coded as final_answer = correct_answer if chosen_strategy == 1 else your_answer, and similar in the other cases.
Use ternary related to print output also – This can also be used in print out as well:

print("Which means it is either your answer: {}, or the only other remaining option, {}.".format(
      correct_answer if your_answer == correct_answer else  your_answer,
      other_remaining if your_answer == correct_answer else correct_answer))

Use the newer print('You chose {}'.format(final_answer) syntax – This is the preferred version for new code in Python 2 and for code in Python 3. See Format specification mini language for examples and specification.
Use docstrings, """ ... """, and not comments, # ..., when describing modules, functions, classes and methods. – If you use docstrings they can be used by your IDE or documentation tools, and it is according to the Pythonic way. :-)
Avoid one-time temporary variables – The playagain can be dropped if you use while True: to keep the game going, and change the end to something like if int(raw_input("Play again (0 = no)?")) == 0: break. (Preferrably with a newline before the break, which does break out of the while loop)
Possibly add a loop or validation on the main loop – If you enter something when choosing between playing one game at a time, or play a chosen number of games, you can enter something wrong and the script terminates. This could possibly be enhanced by adding a while loop around it.

Do also note that if you enter non-numeric text your code breaks very easily...  Look into making a general int input function which uses try ... except around the input function.
Introducing the if __name__ == '__main__': idiom – The previous code is often used to make the code reusable as a module, and to have as little code as possible at the top level. Combined with a main() function your code easily be reused and extended.

In your case you could use (possibly extended with the while loop):

def main():
    game_choice = int(raw_input("Play one game at a time (1) or play your chosen number of games with one strategy (2)? > "))
    play_chosen_game(game_choice)

if __name__ == '__main__':
    break

Consider shortening the text length – Your texts are rather worthy, and could benefit from being shortened a little.

Conclusion

Your code does look good for a beginner, and naming is good (with the exception of not using underscores in between words). You could benefit from learning to use the ternary operator (a if b else c), and a slight justification on using functions related to top level code and text lengths.


Best practice-creating a good PDO connection I have this code for updating time of login failure.The other parts of the code are stripped and simplified because my question is related to PDO.Here I have a db_config.php and a function to update time.I want to know is this a best   practice for PDO connection and accessing variables in a function?

db_config.php

<?php
try
{
  $dbh =  new PDO('mysql:host=localhost;dbname=mark1',dbusername,dbpassword);
  $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch(PDOException $e)
{
  echo 'ERROR: ' . $e->getMessage();
}
?>

Code for update time function.

<?php
require('db_config.php')

$login = strtolower($_POST['login']);

$sql = "SELECT * FROM members WHERE email=:email_db";
$result = $dbh->prepare($sql);
$result->bindParam(':email_db', $login);
$result->execute();
$row = $result->fetch(PDO::FETCH_ASSOC);

if($row > 0)
{
  if($row['user_failed_logins'] == 5 && $row['user_last_failed_login'] > (time() - 60) )// Blocks login attempt for 1 min.
  {
    UpdateLoginFailTime($login,$dbh);
  }
  else
  {
    //Do something...
  }

function($login,$dbh)
{
  $time = time();
  $sql_update_login_fail_fileds = "UPDATE members SET user_last_failed_login = :user_last_failed_login WHERE email = :email";
  $result_update_login_fail_fileds = $dbh->prepare($sql_update_login_fail_fileds);
  $result_update_login_fail_fileds->bindParam(':user_last_failed_login', $time);
  $result_update_login_fail_fileds->bindParam(':email', $login);
  $result_update_login_fail_fileds->execute();

  echo "You have been blocked.Please try login after 1 minute"));
}
?>

Please, never use procedural code-style with object oriented libraries.
For php5+: this is not good code-style, please, see article about Object Oriented Programming; for php5-: this is tolerable variant(but I don't remember if php4 has had PDO).

After learning something about OOP, try to read something about MVC(Model-View-Controller) and Design Patterns(ex.: "GOF: Design Patterns")

P.S. Sorry if it seems roughly for you. But if you want to write best code - it's the only way.


Remove duplicates from array and save it to another one So I have been tasked to make an array without duplicated values from another existing array.
So I did it, but I want to know is there some other better way to do that.
Example input/output:
Input: 10, 15, 10, 5, 1, 3
Output: 10, 15, 5, 1, 3
So here is my code.
#include <stdio.h>

int main(void) {
  const int MAX_ARRAY_SIZE = 5;

  int m[MAX_ARRAY_SIZE], p[MAX_ARRAY_SIZE];

  for(int i = 0; i < MAX_ARRAY_SIZE; i++) {
    printf("Enter number: ");
    scanf("%d",&m[i]);
  }
  int k = 0;
  int dup = 0;
  for(int i =0; i < MAX_ARRAY_SIZE; i++) {
    for(int j = i +1; j <MAX_ARRAY_SIZE; j++) {
        if(m[i] == m[j]) {
            dup = 1;
        }
    }
    if(dup != 1) {
      p[k++] = m[i];
    }
    dup = 0;
  }

  printf("The new array without repeated values\n");
  for(int i = 0; i < k; i++) {
    printf("%d\n",p[i]);
  }

  return 0;
}

I'm not sure if this is the right and simple way I do that. I want some suggestions.
Thanks in advance. :)

Early termination
After
        dup = 1;

you should break. There's no need to execute the rest of the loop.
Booleans
Consider using <stdbool.h>, making bool dup = false, later assigning it true, and writing if (!dup).
Complexity
In practical terms, an array of five values poses no computational cost. However, if your prof cares about complexity analysis, the "proper" solution to this would need to complete in linear time (rather than your current quadratic time), using something like a hash-set, with pseudocode:
Set *seen = make_set();
for (int i = 0; i < MAX_ARRAY_SIZE; i++)
{
    int m;
    scanf(&m);
    if (!contains(seen, m))
        add(seen, m);
}

for (m in seen)
    printf(m);


Netty connecting to multiple servers I am using netty to connect to multiple servers, query for an item, and aggregate the results in my Client. The code below works, but I am unsure if it is the best way of achieving my goal. Also, I'm not sure how I should be closing the futures. Could anyone provide guidance? I am using Netty 4.0.25.Final.

   public void start() throws InterruptedException
   {
      EventLoopGroup group = new NioEventLoopGroup();
      try
      {
         ArrayList<ChannelFuture> channelFutures = null;
         for (MyServerOject server : servers)
         {
            Bootstrap b = new Bootstrap();
            b.group(group)
                     .channel(NioSocketChannel.class)
                     .option(ChannelOption.TCP_NODELAY, true)
                     .handler(new ChannelInitializer<SocketChannel>() {
                        @Override
                        public void initChannel(SocketChannel ch) throws Exception
                        {
                           ChannelPipeline pipeline = ch.pipeline();
                           pipeline.addLast(new myClientHandler(server));
                        }
                     });

            channelFutures = new ArrayList<>();
            ChannelFuture future = b.connect(server.getHost(), server.getPort()).sync();
            channelFutures.add(future);
         }

         for (ChannelFuture channelFuture : channelFutures)
         {
            channelFuture.channel().closeFuture().sync();
         }
      }
      finally
      {
         group.shutdownGracefully();
      }

Some issues with your code

The biggest is that you are creating a
new ArrayList with each interruption of your server.  You probably
want to allocate that array outside the loop that is creating the clients and then add each future to that look as they are created.

I'd avoid the "sync" in the for loop waiting for things to
get done.  If you need to wait for them to complete, attache a
promise listener to each of the futures with a little lambda
function to do whatever it is you need done when the clients are
finished.

I'd pass shutdown gracefully a time limit rather than just
waiting forever.

Without more of your code, its hard to say what else you have
overlooked.


12 Days of Christmas in Rust I've recently been trying to learn more rust and I have just finished chapter 3 of the Rust book, which has a list of projects to do to practice.

I have tried to write a program that prints the 12 Days of Christmas and I would like to know if it is idiomatic or if any major improvementsc could be made.

Thanks in advance.

const LYRICS: [&str; 12] = [
    "Twelve drummers drumming",
    "Eleven pipers piping",
    "Ten lords a-leaping",
    "Nine ladies dancing",
    "Eight maids a-milking",
    "Seven swans a-swimming",
    "Six geese a-laying",
    "Five golden rings",
    "Four calling birds",
    "Three french hens",
    "Two turtle doves, and",
    "A partridge in a pear tree",
];

fn gen_lyrics(day: i32) -> String {
    let ordinal = match day {
        1 => "1st".to_string(),
        2 => "2nd".to_string(),
        3 => "3rd".to_string(),
        _ => day.to_string() + "th",
    };

    let mut start = format!(
        "On the {} day of Christmas, my true love gave to me:\n",
        ordinal
    );

    for i in (0..day).rev() {
        start.push_str(&LYRICS[11 - (i as usize)]);

        if i != 0 {
            start.push_str("\n");
        }
    }

    start
}

fn main() {
    for i in 1..13 {
        println!("{}\n", gen_lyrics(i))
    }
}

This looks pretty good! I'm still fairly new to Rust myself, so I'm not sure how familiar I am with the idioms beyond those outlined in the book. That said, there's a few improvements I can suggest.

Unsigned integer for day

Since day is never negative, you can use an unsigned integer instead of an i32 here:

fn gen_lyrics(day: usize) -> String

If you use usize specifically, you also avoid the cast to usize here:

start.push_str(&LYRICS[11 - (i as usize)]);

That would then just be:

start.push_str(&LYRICS[11 - i]);

Ordinal suffix match expression

You can avoid some .to_string() calls and repetition of numbers by making the match expression return just the suffix of the ordinal. You can then append that suffix to the number itself in the format!() call:

let ordinal_suffix = match day {
    1 => "st",
    2 => "nd",
    3 => "rd",
    _ => "th",
};

let mut start = format!(
    "On the {}{} day of Christmas, my true love gave to me:\n",
    day,
    ordinal_suffix
);

Inclusive range

You can use ..= instead of .. to create an inclusive range:

fn main() {
    for i in 1..=12 {
        println!("{}\n", gen_lyrics(i))
    }
}

Iterators and the skip method

You can iterate directly over the LYRICS array, skipping over the number of lines you don't want to include:

for line in (&LYRICS).iter().skip(12 - day) {
    start.push_str(line);
    start.push_str("\n");
}

The problem with this is that there's no longer an i variable to check whether an iteration is the last line or not, so it can't conditionally insert a newline. The code above will always insert a newline.

But we can cheat a little

If you remove the newline from the end of the start variable:

"On the {} day of Christmas, my true love gave to me:"

You can push the newline first, that way you no longer need a conditional in the loop:

for line in (&LYRICS).iter().skip(12 - day) {
    start.push_str("\n");
    start.push_str(line);
}

Bringing it all together

This is the code with all the changes applied:

const LYRICS: [&str; 12] = [
    "Twelve drummers drumming",
    "Eleven pipers piping",
    "Ten lords a-leaping",
    "Nine ladies dancing",
    "Eight maids a-milking",
    "Seven swans a-swimming",
    "Six geese a-laying",
    "Five golden rings",
    "Four calling birds",
    "Three french hens",
    "Two turtle doves, and",
    "A partridge in a pear tree",
];

fn gen_lyrics(day: usize) -> String {
    let ordinal_suffix = match day {
        1 => "st",
        2 => "nd",
        3 => "rd",
        _ => "th",
    };

    let mut start = format!(
        "On the {}{} day of Christmas, my true love gave to me:",
        day,
        ordinal_suffix
    );

    for line in (&LYRICS).iter().skip(12 - day) {
        start.push_str("\n");
        start.push_str(line);
    }

    start
}

fn main() {
    for i in 1..=12 {
        println!("{}\n", gen_lyrics(i))
    }
}


Parsing error messages using regular expressions in Java 8 I have a below method in Java 8 which checks for message pattern and finds the line number from the message.

 private LawfirmInvoice getInvoiceForErrormessage(InvoiceParcel parcel, String message) {
    if (parcel.getInvoices() != null) {
        if (parcel.getInvoices().size() == 1)
            return parcel.getInvoices().get(0);
        else {
            String regex1 = "^Line\\s+\\:\\s+\\d.+";
            String regex2 = "^Line\\s\\d\\s\\:\\s+.+";
            Pattern p = Pattern.compile("\\d+");
            Matcher m;
            int lineNumber = -1;
            if (message.matches(regex1)) {
                m = p.matcher(message);
                if (m.find()) {
                    lineNumber = Integer.parseInt(m.group());
                } else if (message.matches(regex2)) {
                    m = p.matcher(message);
                    if (m.find()) {
                        lineNumber = Integer.parseInt(m.group()) - 2;
                    }
                }

            }
            if (lineNumber != -1)
                return parcel.getInvoices().get(lineNumber);
        }

    }
    return null;
}

Example : message can be of below types
msg1 = "Line : 1 Invoice does not foot Reported"
msg2 = "Line 3 : Could not parse ADJUSTMENT_AMOUNT value"

The above method finds the line number from the message and returns the instance of invoice from the arraylist.

Since Java 8 has some new features am wondering whether my code can be optimized further.

First, let's optimize what you already have. Extract the regexes, and simplify the logic:

private static final String LINE_EX = "^Line\\s(?:(\\d+)\\s)?\\s*:\\s+(\\d+)?.+"
private static final Pattern LINE_PARSE = Pattern.compile(LINE_EX);

private LawfirmInvoice getInvoiceForErrormessage(InvoiceParcel parcel, String message) {
    if (parcel.getInvoices() == null) {
        return null;
    }

    if (parcel.getInvoices().size() == 1) {
        return parcel.getInvoices().get(0);
    }

    Matcher mat = LINE_PARSER.matcher(message);
    if (!mat.matches()) {
        return null;
    }
    String group = mat.group(1) != null ? mat.group(1) : mat.group(2);
    if (group == null) {
        // neither regex group had digits....
        return null;
    }
    int lineNumber = Integer.parseInt(group);
    return parcel.getInvoices().get(lineNumber);
}

OK, the regex is more complicated, but, there's only one of them, and it is compiled only once. The regex looks for digits on either side of the colon, and returns them in either group 1, or group 2. Note that the regex expression (?: .... ) is a non-grouping match, so does not count as a group, but the (\\d+) inside it, does.

Also, note that your original regex probably has a bug, because it searches for just a single-digit in it: String regex2 = "^Line\\s\\d\\s\\:\\s+.+"; when it should probably look for \\d+ and not just \\d.

Additionally, there is no need to escape the : in your regexes.

Now, there's only 1 regex, not three. It is statically compiled, and our code logic is streamlined.

What we now have, is a function, it takes two arguments, and returns a third. I cannot see any references to 'state' in your class, so your method can easily be a static method. This makes your function a proper java.util.function.BiFunction function if you want it to be, e.g.:

BiFunction<InvoiceParcel, String, LawfirmInvoice> GET_ERROR_INVOICE =
               MyClass::getInvoiceForErrormessage;

This is where you can start having 'fun' with java 8 streams and so on:

final List<InvoiceParcel> parcels = ......

// create an array of all LawFirmInvoices that have errors...
parcels.stream()
    .flatMap(parcel -> getMessagesFor(parcel)
         .stream(message -> getInvoiceForErrormessage(parcel, message))
    .filter(invoice -> invoice != null)
    .toArray(size -> new LawFirmInvoice[size]);

(Assuming a getMessagesFor(parcel) exists....)


Find a serial port device through WMI (windows management instrumentation) The idea here is to be able to find a USB serial port device connected during runtime, thus not knowing its port number, and use it in the application to retrieve information from the device.

string comportInfo = string.Empty;

using (var entitySearcher = new ManagementObjectSearcher("root\\CIMV2", "SELECT * FROM Win32_PnPEntity WHERE Caption LIKE '%" + SerialPortToFind + "%'"))
{
    using (var serialPortSearcher = new ManagementObjectSearcher("root\\WMI", "SELECT * FROM MSSerial_PortName"))
    {
        //testBase.SamplerWithCancel is like a for loop with exception control, a time between interations and amount of iterations to be made. It expects a true or false value to determine whether a desired condition is met. Failing to return a true value in the specified iterations throws an exception.
        testBase.SamplerWithCancel(() =>
            {
            var portList = serialPortSearcher.Get().Cast<ManagementBaseObject>().ToList();
            var matchingEntities = entitySearcher.Get().Cast<ManagementBaseObject>().First();

            if (portList.Count != 0 && matchingEntities != null)
            {
                foreach (ManagementBaseObject port in portList)
                {
                    if (port["InstanceName"].ToString().Contains(matchingEntities["DeviceID"].ToString()))
                    {
                        comportInfo = port["PortName"].ToString();
                    }
                }
                return true;
            }
            else
                return false;
        }, "Serial port not found", 3, 1500, 500, false, false);
    }
}

The code works well, but I want to know where I can improve it to make it more resilient and less error prone.

I feel like using LINQ would be way more appropiate than what I did.

The code works well, but I want to know where I can improve it to make it more resilient and less error prone.

A very good start would be to always use braces {} although they might be optional.
By stacking the using blocks you can save some horizontal spacing.
By reverting the condition of if (portList.Count != 0 && matchingEntities != null) you could return early and the else would be (like now) redundant and could be removed which will save some spacing too.
The naming of the variables could need some imporvements too, e.g the plural matchingEntities doesn't match the result of the call to First().
Storing the result of matchingEntities["DeviceID"].ToString() into a variable will speed up things.

Implementing the mentioned points will lead to

string comportInfo = string.Empty;

using (var entitySearcher = new ManagementObjectSearcher("root\\CIMV2", "SELECT * FROM Win32_PnPEntity WHERE Caption LIKE '%" + SerialPortToFind + "%'"))
using (var serialPortSearcher = new ManagementObjectSearcher("root\\WMI", "SELECT * FROM MSSerial_PortName"))
{
    //testBase.SamplerWithCancel is like a for loop with exception control, a time between interations and amount of iterations to be made. It expects a true or false value to determine whether a desired condition is met. Failing to return a true value in the specified iterations throws an exception.
    testBase.SamplerWithCancel(() =>
        {
        var portList = serialPortSearcher.Get().Cast<ManagementBaseObject>().ToList();
        var matchingEntity = entitySearcher.Get().Cast<ManagementBaseObject>().First();

        if (portList.Count == 0 || matchingEntity == null)
        {
            return false;
        }

        string entity = matchingEntity["DeviceID"].ToString();

        foreach (ManagementBaseObject port in portList)
        {
            if (port["InstanceName"].ToString().Contains(entity))
            {
                comportInfo = port["PortName"].ToString();
            }
        }

        return true;

    }, "Serial port not found", 3, 1500, 500, false, false);
}


Converting a Date to Hexadecimal Word I have two functions, one to convert a datetime to a hex word, and one to convert a word to a datetime. I was just wondering if there was a more efficient way to convert back and forth.

/// <summary>
/// Converts a date time to a hexadecimal word
/// </summary>
/// <param name="date">the date time</param>
/// <returns>returns the Hex value of the date</returns>
private int convertToWordFromDate(DateTime date)
{
  int m, d, y;

  int month = date.Month;
  int day = date.Day;
  int year = int.Parse(date.Year.ToString().Substring(2, 2));

  int yearShift = 9;
  int monthShift = 5;
  int word;

  y = year << yearShift;
  m = month << monthShift;
  d = day;

  if (year == 0)
    word = m + d + (y * 100);
  if (year < 10)
    word = (m + d + y) * 16;
  else
    word = m + d + y;

  return word;
}

/// <summary>
/// Converts a hexadecimal word to a valid dateTime
/// </summary>
/// <param name="word">the word</param>
/// <returns>the date time</returns>
private DateTime convertToDateFromWord(int word)
{
  int month, day, year;
  int monthMask = 0x01E0;
  int dayMask = 0x001F;
  int yearMask = 0xFE00;
  int monthShift = 32;
  int yearShift = 512;
  DateTime date = new DateTime();

  try
  {
    year = (word & yearMask) / yearShift;
    month = (word & monthMask) / monthShift;
    day = (word & dayMask);

    if (year >= 80 && year <= 99)
      year += 1900;
    else if (year >= 0 && year <= 79)
      year += 2000;
    else if (year < 0 || month <= 0 || day <= 0)
    {
      year = 1;
      month = 1;
      day = 1;
    }
    else
    {
      year = 1;
      month = 1;
      day = 1;
    }
    date = new DateTime(year, month, day);
  }
  catch
  {
    date = new DateTime(1, 1, 1);
  }

  return date;
}

I note that your functions are not doing what they say they're doing. For example, you don't convert a date to a hexadecimal word, you convert to an integer value, a value that also happens to disregard the century and the time component entirely. In getting the date back, you also perform some business rules against the date, by treating certain years as last century, other years as current century.

What I would expect when seeing these functions is that I would certainly get the same date I passed into one function as the output from the other. That is not necessarily going to be the case.

You also have issues with a blanket catch that swallows any exception. If you are going to catch an exception, be specific and catch what you truly can handle, something you might expect. Work towards eliminating those exceptions entirely, so instead of catching them, you are preventing them by validating against their causes beforehand, if possible.

You have an if/else chain where an else if and else have the exact some code in their code blocks. Eliminate the redundancy.

There are issues with shifts and masks not being consistent, in terms of where they are defined and the methodology being used. If you change something in one place, you have to change it in another. More than that, you have to change it in a different way.

To be blunt, it's basically not clear what the code is doing at an initial glance, nor is it necessarily clear why it is doing it.

I don't know what your business case is, but if I was writing a class to convert a date to a "hexadecimal word" and then back again, I would probably expect to write something that takes a date and returns a string, and then takes a string and returns a date. The quick, not completely tested version might be something simple like this

public class DateConverter
{
    public string ConvertToHexString(DateTime date)
    {
        return date.Ticks.ToString("X2");
    }

    public DateTime ConvertFromHexString(string hexInput)
    {
        long ticks = Convert.ToInt64(hexInput, 16);
        return new DateTime(ticks);
    }
}

Which you could validate with

DateTime originalDate = new DateTime(1955, 11, 11, 22, 4, 0);
DateConverter converter = new DateConverter();

string hexValue = converter.ConvertToHexString(originalDate);
DateTime returnedDate = converter.ConvertFromHexString(hexValue);

Debug.Assert(originalDate == returnedDate);


Script to create shell aliases Can anybody find anything to pick apart in this script?

#  Copyright (c) 2019. Legorooj

import sys
import os

docstring = '''
add_alias [alias] [command]'''
if len(sys.argv) <= 1:
    print(docstring)
else:
    alias, cmd = sys.argv[1], sys.argv[2]
    print('Adding alias...')
    os.system(f'echo "alias {alias}={cmd}" >> ~/.bashrc && source ~/.bashrc')
    print('Alias added')

If ran like:

add_alias bob "echo bob", it adds an alias as if you ran

echo "alias bob="echo bob"" >> ~/.bashrc && source ~/.bashrc

We need to do some basic sanity checking of the inputs - alias names can't contain whitespace, for example, and any newlines in the expansion will be treated as the end of the alias definition (and the following text will then be a new command in your .bashrc).

The source ~/.bashrc in the call to os.system() looks pointless, as that shell terminates immediately after.


Inspecting properties of files Function Enumerate-Properties($fileName)
{
    $path = (Get-Item $fileName).FullName
    $shell = New-Object -COMObject Shell.Application
    $folder = Split-Path $path
    $file = Split-Path $path -Leaf
    $shellfolder = $shell.Namespace($folder)
    0..287 | Foreach-Object {'{0} = {1}' -f $_, $shellfolder.GetDetailsOf($null, $_)}
}
Function Find-Property($fileName, $PropertyName)
{
    $shell = New-Object -COMObject Shell.Application
    $path = (Get-Item $fileName).FullName
    $folder = Split-Path $path
    $file = Split-Path $path -Leaf
    $shellfolder = $shell.Namespace($folder)
    $shellfile = $shellfolder.ParseName($file)
    $found = $false
    0..287 | Foreach-Object {
        $test1 = $shellfolder.GetDetailsOf($null, $_)
        if($PropertyName -eq $shellfolder.GetDetailsOf($null, $_))
        {
            $found = $true
            return $_
            break
        }
    }
    if(!$found)
    {
        Write-Host "Property "$PropertyName " was not found"
    }
}
Function Get-PropertyValue($fileName, $property)
{
    $shell = New-Object -COMObject Shell.Application
    $path = (Get-Item $fileName).FullName
    $folder = Split-Path $path
    $file = Split-Path $path -Leaf
    $shellfolder = $shell.Namespace($folder)
    $shellfile = $shellfolder.ParseName($file)
    return $shellfolder.GetDetailsOf($shellfile,$property)
}
Function Create-List3($files, $property)
{
    $array = @()
    $propertyNum = Find-Property $files[0] $property
    foreach($file in $files)
    {
        $file | Add-Member -MemberType noteproperty -Name $property -Value (Get-PropertyValue $file $propertyNum).toString() -force
        $array = $array + $file
    }
    return $array
}

This code is designed to be used in something like the following fashion:

$files = dir *.mp4
$propery = "Bit rate"
$array = Create-List3 $files $property
$array | Format-Table Name, Length, $property -auto

As far as I can tell, this code works when you give it a nice amount of .mp4 files. It fares less well when handed other things (folders and the like), the break statement in the Find-Property does nothing (and there has to be a better way to do that anyway).

I am mostly concerned that while this is valid Powershell code, it is very poor Powershell code, in that it is written in a psudo-C# style (that is where I am coming from) and not a Powershell style.

Additionally, I am positive that there easier ways to set up the COM Objects that I am using, and parsing filenames (and reusing them, but that is more complex probably).

How can I make this code better idiomatic Powershell?

A first thing to note is that you are creating a lot of COMObjects in a similar manner, we can simply extract that behavior as a separate function. Then, we note that you are expecting file names as parameters where you could expect an item instead; that allows you to pass on items to Create-List3 instead, thus allowing for piping Get-ChildItem into rather than doing extra conversion calls back and forth.

As a result of this two things we're left with a lot of Split-Path calls, we can simply port them over to extracted function as well, it appared that you only needed one of the Split-Pat calls only once. After that, some small cleanup has been done (removing $test1, adding extra newlines and spaces for readability) and we already have much cleaner code to continue from:

Function Get-ShellFolder($fileItem)
{
    $folder = Split-Path $fileItem.FullName

    $shell = New-Object -COMObject Shell.Application
    return $shell.Namespace($folder)
}

Function Enumerate-Properties($fileItem)
{
    $shellfolder = Get-ShellFolder $fileItem

    0..287 | Foreach-Object {'{0} = {1}' -f $_, $shellfolder.GetDetailsOf($null, $_)}
}

Function Find-Property($fileItem, $PropertyName)
{
    $shellfolder = Get-ShellFolder $fileItem

    $found = false
    0..287 | Foreach-Object {
        if ($PropertyName -eq $shellfolder.GetDetailsOf($null, $_))
        {
            $found = true
            return $_
            break
        }
    }

    if (!$found) {
        Write-Host "Property " $PropertyName " was not found"
    }
}
Function Get-PropertyValue($fileItem, $property)
{
    $shellfolder = Get-ShellFolder $fileItem

    $file = Split-Path $fileItem.FullName -Leaf
    $shellfile = $shellfolder.ParseName($file)

    return $shellfolder.GetDetailsOf($shellfile, $property)
}
Function Create-List3($files, $property)
{
    $array = @()
    $propertyNum = Find-Property $files[0] $property
    foreach($file in $files)
    {
        $file | Add-Member -MemberType noteproperty -Name $property -Value (Get-PropertyValue $file $propertyNum).toString() -force
        $array = $array + $file
    }
    return $array
}

From here on, you could look for things that are already implemented in Powershell to make the code even more simple, as well as adding documentation to make some less obvious things easier to understand for fellow readers.


Processing array of objects into two strings I have an array of Item objects that each have a string particular and a numeric amount.  I want to put all particulars  (separated by newlines) into a single string and the same for all amounts.

Here's what I came up with:

particulars = []
amounts = []

items.each do |item|
  particulars << item.particular
  amounts << item.amount
end

particulars_string = particulars.join("\n")
amounts_string = amounts.join("\n")

So if

item1.particular = "food"
item2.particular = "drink"
item1.amount = 1000
item2.amount = 2000

then running the code above gives

particulars_string # "food\ndrink"
amounts_string # "1000\n2000"

which is correct.  However, I feel that the above code can be done better.  In particular, I want all the code in one loop, not the three (each and two joins) I have now.  What's a better way to do this?

You can get by with just two lines:

particulars_string = items.map(&:particular).join("\n")
amounts_string     = items.map(&:amount).join("\n")

Enumerable#map, which is mixed into Array, creates a new array from an existing array by running a block on each element and storing the result. In this case though you don't need a full block, since you just need to call a single method (particular or amount). So it basically extracts those into new arrays and then joins those arrays.

As a rule of thumb, you almost never have to "manually" map stuff from one array to another in Ruby using each and <<. Be sure to read the docs for Array and Enumerable as they're full of good stuff.


Formatting a date in ClojureScript I want to convert strings with the iso date format (2015-08-02) to the default format of our app (2 Aug 2015) in ClojureScript. As I don't need anything else, I don't want to pull in libs like cljs-time.

I'm quite new to Clojure & ClojureScript, so I want to ask for some feedback on this small snippet. Thanks!

(defn format-date [date]
  (let [month-names [nil "Jan" "Feb" "Mar" "Apr" "May" "Jun" "Jul" "Aug" "Sep" "Oct" "Nov" "Dec"]
        [year month day] (map #(js/parseInt %) (str/split date #"-0?"))]
    (str day " " (month-names month) " " year)))

Very nice work! My only suggestions are to split your large-ish function into several smaller, simpler pieces, and to give each of those pieces its own docstring:

(require '[clojure.string :as str])

(def month-names
  "A vector of abbreviations for the twelve months, in order."
  ["Jan" "Feb" "Mar" "Apr" "May" "Jun" "Jul" "Aug" "Sep" "Oct" "Nov" "Dec"])

(defn month-name
  "Returns the abbreviation for a month in the range [1..12]."
  [month]
  (get month-names (dec month)))

(defn parse-iso-date
  "Returns a vector of the year, month, and day from an ISO 8601 date string."
  [date]
  (mapv #(js/parseInt %) (str/split date #"-0?")))

(defn format-date
  "Converts an ISO 8601 date string to one of the format \"(D)D Mon YYYY\"."
  [date]
  (let [[year month day] (parse-iso-date date)]
    (str day " " (month-name month) " " year)))

This is considerably longer, but most of that additional length is from the fact that now more things are named and documented. The advantage of this approach is that if you find later that you want to use only one or two of these individual parts, or use them in a different way (e.g. when writing another function for a different date format), you can do so without having to duplicate the functionality in your format-date function.

However, I was a bit confused by your comment about libraries:

  As I don't need anything else, I don't want to pull in libs like cljs-time.

One of the greatest features of ClojureScript is that it emits code that is compatible with the advanced mode of the Google Closure optimizing compiler. The impact of this is explained more in the ClojureScript unveiling video (slides are available here). Basically, in ClojureScript, your app can depend on as many libraries as you want, and when you compile it, its final size will only depend on the size of the specific parts of those libraries that you actually end up using.


C++ implementation of recursive_shared_mutex I know it's bad to even use recursive_mutex because of its poor performance, let alone a recursive shared mutex. However, I'm doing this just to practice. Any suggestion will be appreciated!

recursive_shared_mutex.hpp

#pragma once

#include <optional>
#include <shared_mutex>
#include <thread>

class recursive_shared_mutex : private std::shared_mutex {
public:
    recursive_shared_mutex();
    ~recursive_shared_mutex() = default;

    void lock();
    bool try_lock();
    void unlock();

    /*
    * std::shared_mutex's shared locking is recursive by default.
    */
    void lock_shared() {
        std::shared_mutex::lock_shared();
    }

    bool try_lock_shared() {
        return std::shared_mutex::try_lock_shared();
    }

    void unlock_shared() {
        std::shared_mutex::unlock_shared();
    }

private:
    std::mutex mtx_;
    std::condition_variable cv_;
    std::optional<std::thread::id> writer_id_;
    std::size_t writer_cnt_;
};

recursive_shared_mutex.cpp

#include "recursive_shared_mutex.hpp"

recursive_shared_mutex::recursive_shared_mutex()
    : std::shared_mutex(),
      mtx_(),
      cv_(),
      writer_id_(),
      writer_cnt_(0)
{
}

void recursive_shared_mutex::lock() {
    std::thread::id this_id = std::this_thread::get_id();
    std::unique_lock<std::mutex> ulock(mtx_);
    if (this_id == writer_id_) {
        // Same thread/writer trying to acquire the shared_mutex again.
        // Simply increase writer count.
        ++writer_cnt_;
    }
    else {
        // Another writer or no writer is holding the shared_mutex.
        // It's also likely that some readers are holding
        // the shared_mutex.
        if (writer_id_.has_value()) {
            // If another writer is holding the mutex,
            // waiting for it to release.
            cv_.wait(ulock, [this] { return writer_cnt_ == 0; });
        }
        std::shared_mutex::lock();
        writer_id_ = this_id;
        writer_cnt_ = 1;
    }
}

bool recursive_shared_mutex::try_lock() {
    // TODO
    return false;
}

void recursive_shared_mutex::unlock() {
    std::unique_lock<std::mutex> ulock(mtx_);
    if (!writer_id_.has_value()) {
        // No writer is holding the shared_mutex.
        // Call unlock() anyway. UB expected.
        std::shared_mutex::unlock();
    }
    else {
        // At this point, the shared_mutex must be held by a writer
        // and writer_cnt_ must be greater than 0.
        --writer_cnt_;
        if (writer_cnt_ == 0) {
            // If writer count becomes 0, release the
            // underlying shared_mutex.
            // It's likely that we are unlocking in a
            // different thread from the one where the shared_mutex
            // has been acquired.
            // Call unlock() anyway. UB expected.
            std::shared_mutex::unlock();
            writer_id_.reset();
            // Manually unlock to let cv notify.
            ulock.unlock();
            cv_.notify_one();
        }
    }
}

I strongly recommend not inheriting from standard types, and not using private inheritance even with your own types. If a recursive_shared_mutex IS-A shared_mutex, then announce it publicly; if it IS-NOT-A shared_mutex, then don't use inheritance at all!
(Godbolt. D_view here is isomorphic to std::reference_wrapper<D>, for what that's worth.)

std::size_t writer_cnt_;

should be
std::size_t writer_cnt_ = 0;

so that you don't have to remember to initialize it in the member-initializer-list of the constructor. Once you do that, you can =default your constructor. Even if you don't =default the constructor, please don't do mtx_(), cv_(), and so on. Let default constructors do their jobs; and don't force me to read more code than I need to read.

    // No writer is holding the shared_mutex.
    // Call unlock() anyway. UB expected.

This comment is just completely bogus. If this situation is "should never happen, UB expected," then assert false already! Don't just go on and do some random operation like shared_mutex::unlock() when you know you're in a bad state. Assert false, and let the programmer detect and fix their bug.
Again, this follows the mantra "Don't force me to read more code than I have to." Don't make me read a codepath that does shared_mutex::unlock() and returns, when really that codepath is unreachable. In fact, don't even write that if test! Just do this:
void recursive_shared_mutex::unlock() {
    auto lk = std::unique_lock<std::mutex>(mtx_);
    assert(writer_id_.has_value());
    assert(*writer_id_ == std::this_thread::id());
    assert(writer_cnt_ > 0);
    if (--writer_cnt_ == 0) {
        std::shared_mutex::unlock();  // oops, TODO FIXME
        writer_id_ = std::nullopt;
        lk.unlock();
        cv_.notify_one();
    }
}

Notice my preferred use of lk for lock-guard variables.
Notice I'm avoiding the use of ad-hoc named methods like x.reset() in favor of value-semantic operations like x = nullopt.
You've still got a bug here, which you code-commented — but commenting a bug doesn't make it go away! Frankly, this code is literally off-topic here, because it doesn't work and you know it doesn't work.
Figure out how to use a semaphore (from the <semaphore> header in C++20) or an atomic, instead of the shared_mutex which you already know gives you UB.

I'm 90% sure that there's a deadlock possible between your exclusive lock() and unlock() functions. Suppose Thread A has the mutex exclusive-locked, and Thread B goes to lock it. Then Thread B will enter lock(), take an exclusive lock on mtx_, and block in std::shared_mutex::lock() because Thread B still has it locked. (Assume Thread B drops its lock just long enough to fool the cv_.wait() loop.) Finally, Thread A enters unlock() and blocks in ulock(mtx_) because Thread B still has mtx_ locked.