The reason I made getLevelContent a separate method is to avoid multiple for loops for each level of the output (3, 2, and 1). Is separating into methods, as I have done here, a good way to solve the problem?

If it works, it works. You said this is codegolf, so in the end you'll need to determine if it's worth it character-wise to use "extra" methods. Take a look at my response to point 2 and 3 though.

Apart form code golf though, I think "extra" methods help readability. By "extra" methods I mean private methods that are only used once. A better name may be superfluous methods. Say you have some long code that does something easy to understand but you only use it once in one method so it doesn't immediately make sense to extract it to its own method, if it makes the code more readable then I'd suggest extracting it.

  In the printTopographic method, is the for loop more necessary, or more readable, than three separate lines, each calling separately on its own? (I suppose with this loop in place it would be more useful for dozens of lines as opposed to just three.)

A for-loop is better than a separate method call for each line, in fact I'd say a separate call for each line is wrong, see the next one though.

  Are there parts of my code which are just plain inefficient and should/would genereally be written in a different way?

What happens if the input is (1(2(3(4))))? I don't think it's good to hard code the number of levels. Just because the example has three levels doesn't mean every input will have 3 levels. So if you want to do this right I would do something like putting the '(' on a stack to tell what level you are on, the highest number of '(' on the stack is the total number of levels, plus one if 1(2) is allowed. Pop '(' when ')' is met. Look up stuff on converting infix notation (3 * (1 + 2))) to postfix (1 2 + 3 *) for hints on how to do this.

  In projects like this, where I am simply testing what happens and how, I have completely gotten used to labelling static methods, pretty much out of laziness than anything else (for when calling from other methods). Is this a bad habit?

It's more important to understand what the difference between a static method and non-static method is than worrying about always using static in small projects like this.

A method being static means a few things, first is

static methods are on the class itself, not an instantiated object

and because of that

static methods are the same across all instances of the object

which may or may not matter depending on the context. (If you have what should be a "utility class" then it shouldn't be instantiated, see this where I explain what a utility class is.)

For small projects like this that are, for lack of a better term, more like making a script than a large program, most of the methods will end up being able to be static. So if you are making them static because they can be static then you are doing fine. In my opinion if a method can be static it should be static. If you are making them static because you have an obsession with static methods then you're probably missing something. static is not preferable to non-static if you need to do something related to the instance of the object. Perfect example, the idea of getters and setters...

class NumberHolder {
    static int num;
    public static int getNum() {
        return num;
    }
    public static void setNum(int num) {
        this.num = num;
    }
}

// ...

// numberHolders is a List<NumberHolder> with size 10
for (int i = 0; i < numberHolders.size(); i++) {
    numberHolders.get(i).setNum(i);
}
for (int i = 0; i < numberHolders.size(); i++) {
    System.out.println()numberHolders.get(i).getNum(i));
}

will output

9
9
9
9
9
9
9
9
9
9

Which, depending on the context and the problem might be what you want to happen, but personally seeing something like that I would expect

0
1
2
3
4
5
6
7
8
9


When to catch/wrap an exception vs declaring that method throws exception

Programming errors should, as much as possible, produce unchecked rather than checked exceptions. Checked exceptions are for circumstances that will occur during the normal operation of the program and thus should be handled. For example, you should handle the IOException raised by attempting to open a file that isn't there. However, there is no reason to handle the exception raised by a misspelled property name. Programming errors in general should produce unchecked exceptions.

In this case, you should catch all those exceptions inside this function and rethrow them wrapped in a RuntimeException. Don't declare any exceptions types, those are only for exceptions that your caller should handle.


Multiple conditions for same table in EF

Here is an approach to get rid of some of the redundancy:

First write an extension Method

static IQueryable<Product> FilterForParameters
    (this IQueryable<Product> query, string price, string weight, string dimensions, string trend)
{
    if (price != null)
        query = query.Where(p => p.Price.ToLower().Equals(price.ToLower()));

    if (weight != null)
        query = query.Where(p => p.Weight.ToLower().Equals(weight.ToLower()));

    if (dimensions != null)
        query = query.Where(p => p.Dimension.ToLower().Equals(dimensions.ToLower()));

    if (trend != null)
        query = query.Where(p => p.Trend.ToLower().Equals(trend.ToLower()));

    return query;
}

Then change your method

public IQueryable<Product> GetProductsByPara(string price, string weight, string dimensions, string trend)
{
    var products = dbContext.Products;

    var productsFilterdByAll = products.FilterForParameters(price, weight, dimensions, trend);
    if (productsFilterdByAll.Any())
        return productsFilterdByAll;

    var productsFilterdByAllButDimnensions = products.FilterForParameters(price, weight, null, trend);
    if (productsFilterdByAllButDimnensions.Any())
        return productsFilterdByAllButDimnensions;

    var productsFilterdByAllButPrice = products.FilterForParameters(null, weight, dimensions, trend);
    return productsFilterdByAllButPrice;
}

Depending on the context, it would also be a good idea to check your parameters for null at the beginning of the method.


Your DynamicArray class is mimicking an ArrayList which is exactly that: a dynamic array. Use this instead of reinventing the wheel.

Use break; instead of System.exit(0) to exit a loop. It leaves room to add more instructions beyond the loop without having to change the implementation. It's also a more natural way to break out of a loop.

getUser() and getAI() indicate a return type of User and AI respectively. Choose naming that indicates you are receiving their input.

Write your parameter names in full (user instead of u, name instead of n).

Adhere to the single responsibility principle. Printing output and receiving input are two distinct tasks and shouldn't be done in the same method.

Concerning OO principles: perhaps create an interface Conversationist that defines a method String getSentence(). Now your User can implement this as receiving console input while the AI implements it by taking a random value from its stored text. This makes sure you can just call getSentence() for both objects and introduces some inheritance structure.


Index of letter removed to make string palindrome

Flow

The continue and returns inside the while loop make me feel the loop is doing too much.  I would say that the loop should be for moving start and end and then put the logic outside of it:

while ( start < end and string[start] == string[end] ):
    start, end = start + 1, end - 1

# now figure out the result

Result

The problem states that all strings that are not already palindromes are only off by one character.

Our loop moved start and end such that if start == end, then the string is a palindrome.  It has also moved these indices such that if the string is not a palindrome, one of them is 'pointing' to the character that needs to be removed.

To figure out which index is correct, I chose to compare the character pointed to by start to the character pointed to by end - 1.

if ( start == end ):
    return -1
elif ( string[ start ] == string[ end - 1 ] ):
    return end
else:
    return start

Palindrome Function

You don't need this function at all because we know that if the string is not a palindrome, it is only off by one character.  We were able to figure out the index of this character just by moving start and end

Anyway, since the function returns a boolean, it should be named isPalindrome


Binary Search Implementation for Integer Array

Integer division does the right rounding. There is no need to bring in a double machinery.
Search and related algorithms are cleaner on the semi-open intervals (that is, the right boundary is one past the last element).
Testing first and last element looks like a premature optimization. In overwhelming number of cases it is just a waste of time.
Java does not detect and/or eliminate tail recursion. The algorithm can and should be made iterative.

Edit: tail recursion elimination.

Once you found that the recursion is really a tail one, the rest is fairly mechanical.

Rewrite the code to explicit tail form:

if (left >= right) {
    doWhateverNotFoundActionYouWant();
}

middle = left + (right - left)/2;
midValue = value[middle];

if (target < midValue) {
    right = middle;
} else if (target > midValue) {
    left = middle + 1;
} else {
    return middle;
}
return recurse(value, left, right);

Remove the recursion, and replace if with while (don't forget to invert the condition):

while (left < right) {
    middle = left + (right - left)/2;
    midValue = value[middle];

    if (target < midValue) {
        right = middle;
    else if (target > midValue) {
        left = middle + 1;
    } else {
        return middle;
    }
}


Faster binary search to find the index in Python?

I can see the problem with your code. Why This "mid = right // 2  " ?
You can try below code
def binarySearch(arr, low, high, key):
    if high >= low:
        mid = (high + low) // 2
        if arr[mid] == key:
            return mid
        elif arr[mid] > key:
            return binarySearch(arr, low, mid - 1, key)
        else:
            return binarySearch(arr, mid + 1, high, key)
    else:
        return -1


Can I use separate controllers to initialize each Tabs to load the Pane?

Yes. Actually, that's the beauty of Angular. You can do whatever you want.

  Is it good way to give separate controllers for Implementing Pane?

There's no right or wrong. It all depends on what you're trying to do.

Your code looks ok. However, I'd suggest moving your app code into an external JS file. This way, when it grows, it doesn't accumulate in your page.

Multiple controllers are fine. Actually, I recommend you to do it, especially when logic between tabs grow. This way, you can easily add a tab (and its controller) into the fray or easily pull one out without affecting the other tabs logic.

One thing to avoid though is making the parent controller a place of sharing among the controllers. In Angular, child controllers can see the parent controller, its data, its functions. The problem happens when your child controllers implicitly depend on stuff that's on the parent controller, i.e. expect some scope variable from the parent present. Best if you explicitly hand down properties across scopes.


Concept

Obligatory XKCD comic, before I begin:

Enforcing password strength by requiring human-unfriendly characters is no longer considered good practice.  Nevertheless, I'll review the code as you have written it.

"Obvious" simplifications

Any code with the pattern if bool_expr: return True; else: return False should be written simply as return bool_expr.
Strings are directly iterable; there is no need to convert them into a list first, using .split().  In other words, the code would work the same if you just wrote:

upper_list = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"

Better yet, you could just use string.ascii_uppercase.
The uppers += 1 counting loop could be written more expressively using the sum() built-in function.  Actually, in this case, since you only care whether uppers > 0, you could just use the any() function.

With those changes, your check_upper() function becomes a one-liner:

def contains_upper(s):
    return any(c in ascii_uppercase for c in s)

I've renamed check_upper() to contains_upper() to make it clear that the function returns True or False.  Also, avoid using variable names, like input, that coincide with names of built-in functions: it could cause trouble if you ever want to use input().

Code duplication

Most of your check_something() functions are identical.  You should generalize, instead of duplicating the code.

from string import ascii_uppercase, ascii_lowercase, digits

def contains(required_chars, s):
    return any(c in required_chars for c in s)

def contains_upper(s):
    return contains(ascii_uppercase, s)

def contains_lower(s):
    return contains(ascii_lowercase, s)

def contains_digit(s):
    return contains(digits, s)

def contains_special(s):
    return contains(r"""!@$%^&*()_-+={}[]|\,.></?~`"':;""", s)

def long_enough(s):
    return len(s) >= 8

Note that I've used a raw long string to help deal with the need for backslashes in the punctuation string.

validate_password()

The check_dict isn't doing anything for you.  You'd be no worse off with five boolean variables. You are also calling each validation function twice.

The & (binary bitwise AND) operator is not quite appropriate here.  The and (boolean AND) operator would be more appropriate. Even though the results appear identical, the execution differs: the logical and allows short-circuit evaluation.

Personally, I'd write it this way, gathering up a list of all of the failure messages:

def validate_password(password):
    VALIDATIONS = (
        (contains_upper, 'Password needs at least one upper-case character.'),
        (contains_lower, 'Password needs at least one lower-case character.'),
        (contains_digit, 'Password needs at least one number.'),
        (contains_special, 'Password needs at least one special character.'),
        (long_enough, 'Password needs to be at least 8 characters in length.'),
    )
    failures = [
        msg for validator, msg in VALIDATIONS if not validator(password)
    ]
    if not failures:
        return True
    else:
        print("Invalid password! Review below and change your password accordingly!\n")
        for msg in failures:
            print(msg)
        print('')
        return False

If the function returns True in one place, then it would be good practice to return False instead of None in the other branch, for consistency.

Free-floating code

It is customary to put if __name__ == '__main__': around the statements in the module that are not inside a function.  That way, you could incorporate the functions into another program by doing import psk_validate without actually running this program.

Calling sys.exit(0) is rarely desirable or necessary, if you structure the code properly.  Here, all you needed was a break.

if __name__ == '__main__':
    while True:
        password = raw_input("Enter desired password: ")
        print()
        if validate_password(password):
            print("Password meets all requirements and may be used.\n")
            print("Exiting program...\n")
            break


The code is good!

I however do have a couple of niggles:

You should use commas to import multiple functions from a module.
This changes the imports at the top to a single import:

from utilities import ask_something, ask_yes_no

== results in a boolean, no need to make on yourself.

>>> ('y' == 'y', 'y' == 'n')
(True, False)

And so you can simplify the reassignment of the loop flag.
If I already can't read half of a string, you may as well keep it all on one line.
Whilst PEP8 says that we should limit the line length to 79 characters,
formatting large chunks of words within this requirement become an eye saw to me.
And so I'd just leave them on one line.

I also don't really like the verbose messages:

  Invalid input for divisor. Divisor must be a whole number. Please try again.

The message "The input must be a whole number." is enough.
Alternately you could just re-ask the question,
which implies there's been an error.


The main problem is the difficulty of abstracting resources. After compilation, they can only be accessed by their unqiue identifiers, accessible through the generated class R which you already use: R.layout.page1_layout.

Step 1

Let's be strict about the well-known concept DRY: If all of your PageN classes share the same content and the only difference is the referenced layout, you can replace all of your classes PageN with this single class:

public class PageFragment extends Fragment {

    private int layout;

    public PageFragment(int layout) {
        this.layout = layout;
    }

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
        View view = inflater.inflate(this.layout, container, false);
        return view;
    }
}

You can now use this code to initialize the fragments:

fragments.add(new PageFragment(R.layout.page1_layout));
fragments.add(new PageFragment(R.layout.page2_layout));
fragments.add(new PageFragment(R.layout.page3_layout));
// etc...

Step 2

If the layout files differ only in a resource and the visibility of a button, you should abstract it further. Let's think about how to structure the information: All we need in our fragment is the layout file (which should always be the same), the @drawable resource and a boolean for the button visibility.

Let's assume you created a new layout file containing the image view with android:id="myImageView" and the button with android:id="myButton". Everything else should be equal from page to page.

public class PageFragment extends Fragment {

    private int drawable;
    private boolean showButton;

    public PageFragment(int drawable, boolean showButton) {
        this.drawable= drawable;
        this.showButton = showButton;
    }

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
        // Always the same layout
        View view = inflater.inflate(R.layout.page_layout, container, false);
        // Apply per-page configuration
        ((ImageView) view.findViewById(R.id.myImageView)).setImageResource(this.drawable);
        if(!this.showButton) {
            view.findViewById(R.id.myButton).setVisibility(View.INVISIBLE);
        }
        return view;
    }
}

The initialization code is not really pretty:

fragments.add(new PageFragment(R.drawable.page1, true));
fragments.add(new PageFragment(R.drawable.page2, false));
fragments.add(new PageFragment(R.drawable.page3, true));
// etc...

Step 3

Last but not least, you could abstract the initialization. That's not as simple as the previous steps and not necessarily an improvement. Depending on the total number of pages, you might want to keep the initialization code from the second step. It's actually a commonly used practice to avoid abstraction in similar situations, in other words, when the effort of storing the data in an appropriate form is unjustifiable.

In my opinion, it might be a good idea not to call fragments.add in every single line (although it causes a little overhead).

PageFragment[] fragments = {
    new PageFragment(R.drawable.page1, true),
    new PageFragment(R.drawable.page2, false),
    new PageFragment(R.drawable.page3, true)
};
for(PageFragment fragment : fragments) {
    this.fragments.add(fragment);
}

You already declared fragments as a List<Fragment>, so this code should work as well:

PageFragment[] fragments = {
    new PageFragment(R.drawable.page1, true),
    new PageFragment(R.drawable.page2, false),
    new PageFragment(R.drawable.page3, true)
};
this.fragments = Arrays.asList(fragments);

Strictly following DRY ("Every piece of knowledge must have a single, unambiguous, authoritative representation within a system"), this approach should be preferred over the last ones.

However, the most important steps are the first ones, don't waste time trying to optimize the initialization.


Please follow PEP8, the Python style guide.

This function returns either True or None:

def check_Lights():
    if button.title == 'off':
        if button2.title == 'off':
            if button3.title == 'off':
                if button4. title == 'off':
                    return True

Make it a proper boolean. And, instead of the arrow shaped writing style, you can rewrite this as a single boolean expression, for example:

return button1.title == 'off' and button2.title == 'off' and ...

As @Caridorc pointed out in a comment, you can do even better:

return all(b.title == 'off' for b in (button, button2, button3, button4))

Don't repeat yourself. The way you create and add new buttons is very repetitive. Create a utility function to reduce the duplication, for example:

def add_new_button(wcoeff, hcoeff):
    button = ui.Button(title = 'off')
    button.center = (view.width * wcoeff, view.height * hcoeff)
    button.flex = 'LRTB'
    button.action = button_tapped
    view.add_subview(button)
    return button


The API looks ok, IMHO, except for this syntax:

Sequence.Generate(10).Generate(...); // huh?

The downside is that for every sequence, I have to write at least two methods. First, method, to generate an actual sequence (FibonacciGenerator()), second method to wire it up to sequence.Generate (Fibonacci<T>()). This looks like a lot of extra code for what essentially could have been:

SequenceExtensions.FibonacciGenerator(...).Take(count);

Some middle ground can be probably achieved if you use Range instead:

Enumerable.Range(0, count) //extend this
          .Fibonacci(3, 4);

This way one method should be enough in most cases. And you can either drop the Sequence class altogether or leave it as a pretty wrapper around Enumerable.Range or similar loop abstraction:

Sequence.Create(10)      //returns basic 0..9 sequence (IEnumerable<int>)
        .Fibonacci(3, 4) //returns Fibonacci sequence, using initial sequence as loop

I am still not convinced that all this boiler plate code is worth the effort though. But oh well. :)

P.S. I think your Fibonacci sequence implementation is a bit unconventional. There is no "step" in classical implementation:

  every number after the first two is the sum of the two preceding ones

So if I were to see Fibonacci(3, 4) in code, I would assume, that 3 and 4 are the first two numbers of the sequence, and that it will go as 3, 4, 7, 11 and not as 3, 3, 7, 10, 17 (3+3 =/= 7).


Lack of comments, type information, and good naming makes this code much harder to read than it should be.

lls is not descriptive, and the s at the end makes it sound like a list of something, when it's just a number.  Since n is not used for anything else, I'd go with that.

The names c and lc are horrible, these arrays are central to the algorithm, and their definitions take up most of the function, but there is nothing to tell us what they are in the program.  The linked article says that the c array represents cumulative costs, and lc[i][j] is the cost of a line containing words i through j, so I would suggest the names cumulativeCost and lineCost.  However, the c in your program also adds the p array from the article, so cumulativeCost doesn't quite cover it.  cumulativeCostAndPath is a bit long, but not too bad.

Since we're only ever interested in either the cost or the path from cumulativeCostAndPath (though they're calculated together), I'd suggest getpath = snd and cumCost = fst as ways of being clearer about what you mean (you could also separate the pieces, but that would be more work).

With these name changes, the first confusing line of the program, (_:ixs) = reverse $ snd (c ! lls) becomes (_:ixs) = reverse $ getpath (cumulativeCostAndPath ! n).

path j i = let (x, p) = c ! (i-1) in (lc ! (i, j) + x, i:p) is also hard to read, mostly because of naming.  It uses 6 names, of which lc is the only one with more than one character, and only i and j have obvious meanings.  I'm going to call x costSoFar and p pathSoFar.  I'm also going to change the let into a where and break it into 2 lines.

path j i = (lineCost ! (i, j) + costSoFar, i:pathSoFar)
           where (costSoFar, pathSoFar) = cumulativeCostAndPath ! (i-1)

It is not obvious what extras is trying to do, even with the article as a reference.  It would be much easier to follow with a comment explaining the formula for extras[i][j] in the article, what extras is meant to represent, and mentioning that the scanl1 call in extras and the tails call in go interact to create the last term in extras.

The costs function could use a comment explaining what each of the 3 possibilities means.  If extras were extracted to be its own array, it would allow costs to be rewritten more cleanly, as well as potentially simplifying the definition of lc.  Before figuring out the details of what's going on, that deep pipeline of functions was hard to reason about.

You spend a lot of effort making lists which have to match up to arrays.  This is harder to follow than defining each array element with a function (which is possible for these arrays), and it sets you up for off-by-one errors if you're not careful.

I can see two possible ways to speed it up.  You use exp in the definition of costs, which is more expensive than the 2 multiplications required for cubing suggested in the article.  Also, the calculation of extras doesn't stop after it's impossible.  Since after finding that words i through j won't fit, all of the rest of them won't fit either, you could avoid calculating everything after the first negative number.  If you have 500 words to wrap and an 80 column limit, figuring out whether you can fit words 1 through 200 on a line is a waste of time.


Buy and Sell a Stock for maximum profit (given a day and a price)

Readability

The way you've named your variables makes the code more difficult to read for example peak just doesn't make sense to me. Another consideration would be to rename buy_day to cheapest_day
Don't overcomplicate things: I found the return statement at the end of your forEach confusing

General syntax improvement that would make the code easier to read:

Use camelCase over snake_case
Use dot notation when accessing an objects properties where possible

Remove unnecessary/unused code

You have an if/elseif that does the same thing this can be simplified using an or statement if (this || that)
peak.sell and peak.sell_day are never used, same for all in your prices.forEach

Misc

There may be some reasoning you had which I'm missing but I did't see the point in accepting the second argument peak.

Rewrite

const perfectPrice = (prices) => {
    const week = ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday'];

    let profit = 0;
    let profitable;

    const cheapest = {
        day: null,
        price: null,
    };

    prices.forEach((price, day) => {
        if (day === 0 || price < cheapest.price) {
            cheapest.price = price;
            cheapest.day = day;
        }

        const potential = price - cheapest.price;

        if (!(potential > profit)) {
            return;
        }

        profit = potential;

        profitable = {
            buy: week[cheapest.day],
            buyAt: cheapest.price,
            sell: week[day],
            sellAt: price,
            profit,
        };
    });

    return profitable || 'not profitable to sell';
};

console.log("(test 1):", perfectPrice([7, 1, 5, 3, 6, 4]))
console.log("(test 2):", perfectPrice([7, 6, 4, 3, 1]))
console.log("(test 3):", perfectPrice([7, 1, 5, 3, 6, 4]))
console.log("(test 4):", perfectPrice([1, 2]))
console.log("(test 5):", perfectPrice([7, 1, 5, 3, 6, 4]))
console.log("(test 6):", perfectPrice([2, 9, 5, 3, 1, 4]))


Loop through different controls and enabled the state of the whole group VBA

I rows are being added dynamically I would write a class to wrap each row of controls and respond to the combobox events.

Since there appears to be is a fixed number of rows, there is no reason to iterate over all the Controls in the userform.

These variables seem legacy code because they are set but never used.

Dim ctl As MSForms.Control
Dim lbl As MSForms.Label
Dim cmb As MSForms.ComboBox
Dim txtbox As MSForms.TextBox
Dim optbtn As MSForms.OptionButton

This code is repeated several times.

If Controls("Item" & i - 1).Value <> vbNullString Then
    Controls("OrderLbl" & i).Enabled = True
End If

The If statements can usually be eliminated when setting Boolean values.

Controls("OrderLbl" & i).Enabled = Controls("Item" & i - 1).Value <> vbNullString

A helper variable would make the code even easier to read.

Enabled = Controls("Item" &  i - 1).Value <> vbNullString
Controls("Qty" & i).Enabled = Enabled
Controls("UnitPrice" & i).Enabled = Enabled
Controls("SubTotal" & i).Enabled = Enabled
Controls("Comments" & i).Enabled = Enabled

Refactored Code

Private Sub UpdateItemsToClean()
    Const ItemCount As Long = 5
    Dim Enabled As Boolean
    Dim Index As Long
    Enabled = True
    For Index = 2 to ItemCount
        If Enabled Then Enabled = Controls("Item" & Index - 1).Value <> vbNullString
        Controls("OrderLbl" & Index).Enabled = Enabled
        Controls("Item" & Index).Enabled = Enabled
        Controls("Qty" & Index).Enabled = Enabled
        Controls("UnitPrice" & Index).Enabled = Enabled
        Controls("SubTotal" & Index).Enabled = Enabled
        Controls("Comments" & Index).Enabled = Enabled
        Controls("OptionIn" & Index).Enabled = Enabled
        Controls("OptionOut" & Index).Enabled = Enabled
    Next
End Sub


Test Driven Development, Mocking & Unit-testing

I see two options:

Test/implement GetCurrentApplicableRenewal first.
Implement the code you need (which will likely go into GetCurrentApplicableRenewal) in your SelectSubscription method now, imeplement/test GetCurrentApplicableRenewal next, and refactor it to use the GetCurrentApplicableRenewal method later.

The latter allows you to implement and define tests for both methods in isolation from one another, only refactoring when you have a full set of tests.  With these tests, you can safely validate whether refactoring SelectSubscription can safely switch to using GetCurrentApplicableRenewal or not.

The former way will save you some time, and as long as you don't skip any tests for SelectSubscription, you will probably be OK.  The downside is that you have to context-switch to GetCurrentApplicableRenewal.


As a side note, you pass the element by copy not by const reference. This might also have some real preformance effects. SO rather do

 void Insert(const T&)

Also you could use emplace_back to enable inplace construction of the elements in the vector. On the other hand std::find only takes const T& so I would guess, that move semantics do not really buy you anything here.


Rotate a matrix, using transposition and reflection

Keep in mind that the primary focus of this web site is to review your working code and provide suggestions on how to improve the code. Questions about which way is better or worse may be based on opinion and that is off-topic for most of the stack exchange websites.
In answer to your question, it could be a possible optimization that reduces the number of steps, but that isn't clear. You may be missing the point of the interview, which is that you should explain as clearly as possible to the interviewer(s) what you are doing at each step so they understand your problem solving techniques. Please note that the video code looked more like C rather than C++, but could have also been other languages.
Avoid Using Namespace std
If you are coding professionally you should get out of the habit of using the using namespace std; statement. In an interview you definitely don't want to use this statement because it is unprofessional. The concept of Name Spaces was invented to specifically solve certain problems such as collision of duplicate function names from core code and library code. This stack overflow question discusses this in more detail.
Use Code Blocks in Flow Control Rather Than Single Statements
Professional coders generally put code blocks or complex statements into if statements, else clauses or loop bodies. The reason for this is to enhance the maintainability of the code. To many bugs have been introduced into code by trying to add one statement to an if statement, else clause or loop. A code maintainer may forget to add the necessary braces so putting them in from the beginning is better. It also makes the code more readable and easier to understand to begin with.
So code like
        int N = mtx.size();
        for (int n = 0 ; n<= N-2; n++)
            for(int m= n+1 ; m<= N-1; m++)
                swap(mtx[n][m], mtx[m][n]);

is better written as
        size_t N = mtx.size();
        for (size_t n = 0 ; n <= N-2; n++)
        {
            for(size_t m= n + 1 ; m <= N-1; m++)
            {
                std::swap(mtx[n][m], mtx[m][n]);
            }
        }

Obviously a maintainer of the code can figure out where to add a statement here.
Horizontal Spacing
It is common to leave spaces between operator and variable. The horizontal spacing in this code is inconsistent.
Prefer std::size_t Over int For Loop Control Variables That Index through C++ Container Classes
While the code in the example from this question will compile and run properly there may be warning messages if the strictest level of compiler switch is used. std::vector.size() returns std::size_t which is unsigned, rather than signed. If the strictest level of switches are used a the following loop might yield the warning message type mismatch.
        for (int n = 0 ; n <= mtx.size() - 2; n++)
        {
            for(int m= n + 1 ; m <= mtx.size() - 1; m++)
            {
                std::swap(mtx[n][m], mtx[m][n]);
            }
        }


Getting data correctly from  tag with beautifulsoup and regex

As it happens, there are positive answers to each of your questions:

with Beautiful Soup you can remove the WebRupee span with replace_with() entirely....

webrupee_element = soup.find('span', {'class': 'WebRupee'})
webrupee_element.replace_with('')

... then, when you get the text value of the product_price_tag_element.text it will not have the symbol.

EDIT: Of course, it would be faster/better to do:

for wr in product_price_tag_element.find('span'):
    wr.replace_with('')

Your regex is not matching the value properly because the &nbsp; character may not be mapping directly to the regular ' ' character in your regex. You should use the 'whitespace' escape-sequence \s in your regex instead of ' ', like reg = ur'^\s*₹?\s*(\d+)\s*'
Unicode string literals in python should be escaped with either the \u or \U escape, but they are different:

  Specific code points can be written using the \u escape sequence, which is followed by four hex digits giving the code point. The \U escape sequence is similar, but expects 8 hex digits, not 4.

in your case you should use lower-case \u
You can remove the utf-8/unicode declaration if you encode the values in the more-pythonic way of unicode escapes.


Good things:

There is a very descriptive docblock above the function that documents the arguments.
const is used for things that don't change, whereas let is used for values that can be re-assigned.

Suggestions

When inserting at the end, just use Array.push() instead of Array.splice(). This not only simplifies the syntax but also should be faster (see this jsperf).
For performance reasons it would be wise to use a for...of loop instead of a forEach() to iterate over the items in source - especially for large arrays. Functional programming is nice but it has drawbacks - especially when performance is concerned. Consider that each iteration has a call to cmp so calling an extra function for each element in source could lead to a lot of extra overhead. With such a change the variable insertAtEnd could likely be removed if a label outside the outer for loop was added and that label was used with a continue statement instead of the break.


SFINAE with several condition [Refactoring]

Seems short enough to me.

It's impossible to know how to refactor any code without first knowing what it is supposed to do. All we know about this no-op function is that you "want [to] exclude several type[s] like void or char". How many types? Which types? Why?

The obvious shortening is

template<class U, class = std::enable_if_t<std::is_fundamental_v<U>>,
    class = std::enable_if_t<!std::is_same_v<char, U> && !std::is_same_v<void, U>>
>
void multiply(U&&) {}

i.e. just remove all the superfluous ::type and ::value, and replace typename by class because it's shorter (and you did ask for us to make it shorter!).

Your naming the function multiply implies that maybe what you want is

template<class U>
auto multiply(U&& u) -> decltype(u*u, void()) {}

Your comments about "don't must be number" imply that maybe what you want is

template<class U, class = std::enable_if_t<std::is_arithmetic_v<U>>>
void multiply(U&&) {}

But we really can't know for sure unless you tell us what the code is supposed to do.


A clean and efficient makefile for a simple c program

clean and all are not files should therefore be declared as phony.

.PHONY: clean all
all : $(PROG1) $(PROG2) $(PROG3)
...
clean :
    /bin/rm -f *.o a.out $(PROG1) $(PROG2) $(PROG3)


Don't use Hungarian notation

The compiler doesn't need it and doesn't check it, and for a human it is all too easy to accidentily add the wrong prefix and get confused later. So no ptr in front of pointer variables.

Array variables can be used as pointers.

In the following code:

char string[] = "This is a test";
char* ptrString = string;

You don't need the second variable at all. Just use string directly.

Include the right header files

The function strlen() is officially defined in <string.h>, so make sure to #include that at the top of your program. Sometimes your program works, maybe even without compiler warnings, but you cannot assume that it will keep working in the future or that you can port it to other systems if you don't.

Don't include headers you don't use

You include headers at the top of helloworld.h that you are not using in that file. You should remove them.

Protect your header files from multiple inclusion

When you have a larger project, it can happen that your own header files need to #include some of your other header files. It can then become unavoidable that a header files inadvertantly gets included twice. To prevent any errors from happening, use the following trick to allow a header file to be included multiple times:

#ifndef __HELLOWORLD_H__
#define __HELLOWORLD_H__
...the real contents of helloworld.h here...
#endif

Use better names

Variables and structs should have names that are clear and to the point. Naming something SomeStringStruct is not to the point, it sounds vague and handwavy. Choose something better. The same goes for ptrAnyStringStruct.

Use size_t for lengths and sizes

The proper type to store the length of a buffer or a string is size_t. If you read the manual page of strlen(), you will also notice that it returns a value of type size_t.

Use the correct format specifier for size_t

Use %zu instead of %lu when you are printing a variable of type size_t.
If %zu is not supported by your compiler, then cast the variable to the right type when printing it; for example with %lu, cast it to long unsigned int:

printf("string length: %lu\n", (long unsigned int)strlen(string));

Be aware though that on some systems (notably 64-bits Windows), long int can be smaller than size_t, and if you would ever have a string longer than can be represented in a long int, the output will be incorrect.


A from_string() function (inverse of std::to_string)

Just going to review your original code and not the edit.

inline std::istringstream& get_istringstream(){
    static thread_local std::istringstream stream;
    stream.str("");
    return stream;
}

template <typename T>
inline T from_string(const std::string& s){
    auto& iss(get_istringstream());
    iss.str(s);
    T result;
    iss >> result;
    return result;
}

As Jerry Coffin mentioned, you are basically reinventing boost::lexical_cast.  I wasn't sure if I wanted to review this or flag it as broken code, but it technically compiles and it technically works for one conversion (perhaps a discussion for meta-cr).  Test your code thoroughly.

  Are there other considerations I've overlooked, performance-wise (*) or usability-wise?

You don't handle errors.  Consider someone doing this:

auto x1 = from_string<int>("Not an int");

What is the value of x1?

    T result;      // uninitialized variable.
    iss >> result; // operator>> fails silently, sets failbit
    return result; // return uninitialized variable.

The result is undefined behavior since we are returning an uninitialized value that was never assigned to.  Remember to check that the reading of a value was successful.  Notify the callee whenever that an error, like invalid argument or out of range, has occurred.

There are different error handling strategies today.  The common ones are exceptions, std::system_error, and Alexandrescu's expected<T, E> (currently being proposed for standardization).

Let's say someone does something common, like use your function more than once.

auto x1 = from_string<int>("42");
auto x2 = from_string<int>("43");

The first line returns 42, that is fine.  The second line has an issue.

    T result;      // uninitialized variable.
    iss >> result; // eofbit enabled from previous stream consumption
    return result; // return uninitialized variable.

Undefined behavior again.  The original code had

std::istringstream iss;

On construction, isss internals were initialized (data and error state).  In your refactored code, you wanted to mimic a newly constructed object

inline std::istringstream& get_istringstream(){
    static thread_local std::istringstream stream;
    stream.str("");
    return stream;
}

    auto& iss(get_istringstream());

Turns out your refactored code wasn't equivalent.  You forgot to clear the error state.

Consider the following code:

auto x1 = from_string<double>("42,004");

What is the value of x1?  Conversion of arithmetic types using separators is dependent on the locale of the stream.  The possible results are

forty-two thousand and four,
forty two and four one-thousandths, or
forty-two.

If you are fine with this behaviour, document it.  If you want to ensure you always use the behavior of a specific locale by default, consider imbuing before reading.

  Perhaps I should mention my motivation here is partly how I found it strange that there's no std::from_string().

<std::string> provides functions to convert to arithmetic types

Reals - std::stod, std::stold, std::stof
Signed integrals - std::stoi, std::stol, std::stoll, and
Unsigned integrals std::stoul, std::stoull.

You could also consider copying the naming scheme from these and have string_to<T>.

For general purpose conversions, the ones that immediately come to mind are  boost::lexical_cast (stream-based) and boost::spirit (policy-based).  Boost also has boost::convert, an adapter library built to work with various conversion utilities (strtol, boost::spirit, boost::lexical_cast, and C++-Streams).


Train reservation system in Rust as my practice

let time: &[&str] = &["12:00", "14:00", "18:00", "19:00"];
let mut stations = HashMap::new();
stations.insert(
    reserve_request::Stations {
        start: "Tokyo",
        destination: "Kyoto",
    },
    time,
);

Ok, I assume you tried putting the value assigned to time directly in the expression, but were unable to resolve the resulting error message. The problem is that expression has the type [&str; 4], an array. But you want a slice: &[&str] instead. There a few ways to resolve this:

Firstly, you can specify the type of the HashMap:

let mut stations: HashMap<reserve_request::Stations, &[&str]> = HashMap::new();
stations.insert(
    reserve_request::Stations {
        start: "Tokyo",
        destination: "Kyoto",
    },
    &["12:00", "14:00", "18:00", "19:00"]
);

This way, Rust will infer what type you actually wanted at convert the reference to an array into a slice.

Alternatively, you can manually request a slice by called .as_ref() on the array:

let mut stations = HashMap::new();
stations.insert(
    reserve_request::Stations {
        start: "Tokyo",
        destination: "Kyoto",
    },
    ["12:00", "14:00", "18:00", "19:00"].as_ref()
);

However, a better approach would be to not use arrays at all. Rust arrays are really just not that useful, and you almost always should prefer to use a Vec. Furthermore, if you implement loading from JSON as your comment suggests, then you'll certainly get a Vec and not a slice.

let mut stations = HashMap::new();
stations.insert(
    reserve_request::Stations {
        start: "Tokyo",
        destination: "Kyoto",
    },
    vec!["12:00", "14:00", "18:00", "19:00"]
);

Moving on the second part where you had an issue:

    let times = timetable.get(&st);
    let default: &[&str] = &[];

Firstly, we have the same problem as before with a (in this case empty) array. It can be solved in the same way as above.

    let result = match times {
        Some(r) => r,
        None => default,
    };

Option has a method, unwrap_or which does the same thing:

    let result = timetable.get(&st).unwrap_or([].as_slice());

However, a better approach would be use an if let statement.

    if let Some(times) = timetable.get(&st) {
        for t in times {
            if *t == self.time {
                return true;
            }
        }
    }

This way the loop over times only runs if it was present and you don't need to create and iterate over an empty slice.

However, if you use Vec instead of slices as I suggest, the inner loop can be written as a call to the contains function:

        if times.contains(&self.time) {
            return true;
        }

Or you can replace the whole function with:

        timetable
            .get(&Stations {
                start: self.start,
                destination: self.destination
            })
            .map_or(false, |times| times.contains(&self.time))

map_or returns false if the time table was not found, otherwise it calls the closure which checks if times contains the appropriate time.


Improving a method to guarantee uniqueness

Having a loop to check each version of the URL could become a problem if there end up being a lot of URL's. There many be a better way to do it using a hash, or some other means of persisting data, and retrieving the most recently used version.

Still, even with your loop code, there's a way to do it in a simpler fashion, using a helper method to check for a duplicate URL is a start.....

private bool PageFound(string baseUrl, string friendlyURL)
{
    var fullUrl = baseUrl + friendlyUrl;
    var duplicateUrlPage = Site.Pages.FirstOrDefault(
                 x => x.FullUrl != null
              && string.Equals(x.FullUrl.Trim(), fullUrl.Trim(),
                       StringComparison.CurrentCultureIgnoreCase));
    return duplicateUrlPage != null;
}

private string GetUniqueUrl(Page parentPage, Page newPage)
{
    var version = 0;
    var baseUrl = parentPage != null ? parentPage.FullUrl.Trim() + "/" : string.Empty;
    var friendlyUrl = newPage.FriendlyURLName;
    while (PageFound(baseUrl, friendlyURL))
    {
        version++;
        friendlyUrl = newPage.friendlyURLName + "-" + version;
    }
    return newPage.FriendlyURLName + "-" + version;
}

There are two things to pay attention to here.

Note how the first time the duplicate is checked, there's no 'version' on the URL? The version only gets added for version >=1. This is 'managed' by doing the first URL construction outside the loop, and the rest inside the loop. This saves the conditional on the version == 0
I believe there's a bug in your code (which I have reproduced in my suggestion). If you search for the page without the version (version = 0) and you do not find one, you return the URL with the version anyway (you append "-0"). Are you sure this is what you want to do? If it is not what you want, you can just return friendlyUrl instead of newPage.FriendlyURLName + "-" + version

Using a helper function to check for the duplicate is a good way to put a code block in to a conditional clause for a loop. In essence, by creating a function, we have also created a powerful while loop check. The 'problem' with your code is that it was trying to do condition-loop logic inside the execution block, instead of inside the loop clause
2.


You can use I18n directly in factory definitions

FactoryGirl.define do
  factory :static_page do
    language "en"
    trait :purchase_loan do
      page_name { I18n.t("static_page.page_name.purchase_loan", locale: language) }
      titlebar_text { I18.t("static_page.titlebar_text.purchase_loan", locale: language) }
    end

    trait :refinance_loan do
      page_name { I18n.t("static_page.page_name.refinance_loan", locale: language) }
      titlebar_text { I18.t("static_page.titlebar_text.refinance_loan", locale: language) }
    end
  end
end

This will allow you to share locales in views / application code and in your tests.

But you could go 1 step further and in your model store only locale keys, ie.

class StaticPage < ActiveRecord::Base
  #attribues in db :pagename, :titlebar, :language
  def pagename_text
    I18n.t(pagename, locale: language)
  end

  def titlebar_text
    I18n.t(titlebar, locale: language)
  end
end

then you can use it this way:

purchase_loan_en = StaticPage.create(language: 'en', page_name: 'static_page.page_name.purchase_loan')

this will allow you to remove translations directly from factories.

Have a look on I18n Rails guide for how to use I18n. Maybe you don't even need to store language in database.


Writing and saving scraped documents in several sheets in an Excel file

Overall I don't think it's terrible. Suggestions:

Add a shebang to the top of your script so that it's executable in Posix-like environments: #!/usr/bin/env python
You create a session every time you call get_docs. Don't do this unless absolutely necessary; instead, pass the session created in get_links to be reused.
Put this through a PEP8 linter, such as that built into PyCharm, for minor formatting suggestions, such as there being two newlines before every top-level function.
Your sheetname (which should be called sheet_name by Python standard) smells like it should be a regex instead of a series of splits.
Condense your if ... in ... else to a setdefault(); something like:

data.setdefault(sheet_name, []).append([title, str(link)])


If any locking on _queueItemLock is happening inside a lock on _queueLock then you can consider this as thread safe.

If any access of _entryCount is done in a lock on _queueLock then you don't need the Interlocked.Increment(ref _entryCount); but you can just increment in the normal way.

If you by any chance are using C# 6.0 (VS 2015) you can use string interpolation by using the $ operator without the need of calling string.Format() like so

Logger.Trace($"Running task queue item for {_methodName}.");

other than that there isn't much to say.


Find the best path amongst N random points

One minor way to improve speed would be not to calculate the square root of all of the distances before comparison, but instead only return the square root of the shortest distance squared because the square root function is costly.


Access function to return "number of working days until date" for form

Don't compare a Boolean expression with a Boolean value to make.. a Boolean expression - the syntax for If blocks goes If {Boolean Expression} Then - therefore, = False is unnecessary here:

If getWorkindDaysUntilDateDict.exists(p_date) = False Then

The IsDateValid function can be simplified:

isDateValidAdd = True

If getHolidayLibrary.exists(p_date) Then
    isDateValidAdd = False
'saturdays
ElseIf Weekday(p_date, vbSaturday) = 1 Then
    isDateValidAdd = False
'sunday
ElseIf Weekday(p_date, vbSaturday) = 2 Then
    isDateValidAdd = False
End If

Is logically equivalent to :

Dim wkDayForDate Integer
wkDayForDate = Weekday(p_date, vbSaturday)

IsDateValidAdd = Not (GetHolidayLibrary.Exists(p_date) Or wkDayForDate = 1 Or wkDayForDate = 2)

Now because VBA will not short-circuit, one may argue that the long-winded If..ElseIf block will perform better... but comparing {Integer} = {value} isn't exactly an expensive operation, so I wouldn't worry about not short-circuiting.

If anything you may want to avoid accessing GetHolidayLibrary when Weekday(p_date, vbSaturday) is 1 or 2, so this:

Dim wkDayForDate Integer
wkDayForDate = Weekday(p_date, vbSaturday)

If Not (wkDayForDate = 1 Or wkDayForDate = 2) Then
    IsDateValidAdd = Not GetHolidayLibrary.Exists(p_date)
End If

Might just be the best compromise.

"Is date valid add" isn't exactly a great name. Assuming it means "can this date be added?", it could be renamed to CanAddDate, or perhaps IsWorkday.

Procedure names should be PascalCase, and I would drop the p_ prefix for parameters and the m_ prefix for fields.

Private Function getHolidayLibrary() As Scripting.Dictionary

    If m_HolidayLibrary Is Nothing Then
        Set m_HolidayLibrary = createHolidayLibrary
    End If

    Set getHolidayLibrary = m_HolidayLibrary

End Function

This looks like you're trying to "encapsulate" the dictionary with a getter... but in the same scope as the rest. You could have a class responsible for this encapsulation, and then the module wouldn't have to care about whether the holidays are loaded; CreateHolidayLibrary (which is really LoadHolidays or, in the case of a hypothetical HolidayLibrary class, simply Load or Refresh) would belong there as well.


Performance

Your code is slow because your algorithm is \$O(n^4)\$, where \$n = 900\$:

The outer x loop runs for 900 iterations (\$O(n)\$).
For every x, the y loop runs for 900 iterations (\$O(n)\$).
For every product, you search the array to see if the entry already exists.  Since the length of the array will grow up to \$O(n^2)\$, and .include? does a linear search, the deduplication takes \$O(n^2)\$ time.

You can cut the time in half by taking advantage of the fact that \$x \cdot y = y \cdot x\$.  The big win, though, would be to store the results in a Set, which performs deduplication for "free".  That would bring your runtime down to \$O(n^2)\$.

Style

The standard indentation width for Ruby is two spaces.

The parentheses around (x * y) are superfluous.  However, parentheses for the parameter to .include? would be considered good practice.


JavaScript-based quiz application called Quizz

I can understand your frustration of nested for loops.

However, your data structure is full of nested arrays. How else can you handle them?

One way to have pretty code is to separate each loop in its own function, with clear names.

This way, the main logic remains readable. And if you want to go in detail, the only scope you have to care about is a small function.

You can refactor more than in the example given above, it's all up to you.

Example:

var digest  = function() {
        for( var q in qbank ) {
            //  q   is js/html/php/..... topic names
            var
                topic,
                t,
                qns     = 0,
                i   = 0,
                questions,
                question,
                level
            ;

            topic   = qbank[q];

            initInstance( _instance, q );

            for( t in topic ) {
                initTopics( _instance, t, q );

            }
        }

        //  Initializing instance topics
        function initInstance( _instance, q ) {
            _instance.topics[q]= [];

            for(
                var
                    l = levels.length
                ; i < l; i++ ) {

                if( typeof _instance.topics[q][i] === 'undefined' ) {
                    _instance.topics[q][i]  = {
                        'level': levels[i],
                        'questions': []
                    };
                }
            }
        }

        function initTopics( _instance, t, q ) {
            var
                questions   = topic[t].questions,
                level       = topic[t].level
            ;

            for( qns in questions ) {
                for( var
                        inst = 0,
                        l = _instance.topics[q].length; inst < l; inst++ ) {

                    if( _instance.topics[q][inst].level === level ) {
                        _instance.topics[q][inst].questions.push( questions[qns] );
                    }
                }
            }
        }
};


Displaying pagination links, maybe with links to first and last page

The main loop code lacks the prior validation. So in the business logic part

verify that $x is positive. if not, make it the least number possible (1 I suppose)
verify that $x is <= $total_pages. Make them equal otherwise
prepare the page numbers in the form of array using $pages = range($initial_num, $condition_limit_num);

So you will have a single loop with a single condition

<?php foreach ($pages as $x): ?>
    <?php if ($x == $page) : ?>

            <?= $x ?> (current)

    <?php else: ?>

            <a href="<?= $page_url ?>page=<?= $x ?>"><?= $x ?>

    <?php endif ?>
<?php endfor ?>


Exercise 6.1 from ANSI C book - getword function

The last logic is quite hard to trace, and it is prone to bugs. Consider /* ... */*. Once the code reached the closing /, it returns, but last is not updated, and remains /, so the next * is still treated as a beginning of a comment.
Handling of string constants has a definite problem: it doesn't account for an escaped quote char in the middle of the string (e.g. "aaa\"bbb").

Combining the two above observations, I strongly recommend to revisit the design, and realize few more functions, such as get_line_comment, get_c_comment, get_quoted_string etc.

No word in keytab starts with an underscore. You don't need to go into binsearch in that case.
The preprocessor directive may not appear in the middle of the line. I am not sure if a directive in such case should be counted.


Your should use csv to read the file.

import csv
with open('words.txt', 'rb') as csvfile:
    words = [
        word
        for line in csv.reader(csvfile, delimiter=',', quotechar='"')
        for word in line
    ]

It's much easier to understand what it is doing. And it's probably safer and faster.

You define letters, however you use string.ascii_uppercase instead.

You can put max_word_score in the same 'block' as the while loop below it.
You can also re-arange 1/2.0*n*(n+1) <= max_word_score to get the max n.

Python also has a style guide called PEP8, which says you should have a space either side of operators. 1 + 1 not 1+1.
The only exception to this is when you want to show precedence. 1 + 2*3.

You can change your for word in words to one large comprehension, and use sum. For better reliability.

max_word_score = len(max(words, key=len)) * 26
triangle_numbers = [
    0.5 * n * (n + 1)
    for n in xrange(int((2 * max_word_score + max_word_score)**0.5))
]

count = sum(
    1
    for word in words
    if sum(letters_map[char] for char in word) in triangle_numbers
)

If you also assume that you are not going to get the longest English word you can change max_word_score = len(max(words, key=len)) * 26 to max_word_score = 1909 * 26, which will allow the second longest English word.
And as they are 'common English words' you could change max_word_score to 27 * 26, which will allow the 9th longest English word.

And if you do use max_word_score = 30 * 26, then you can reduce the memory usage.

You should also change letters_map, max_word_score and triangle_numbers to uppercase variants, e.g. LETTERS_MAP.
This is as they are constants.

If I were to do all the above then I would use the following program:

import csv
import string

MAX_TRIANGLE = 30 * 26
LETTERS_MAP = {letter: idx for idx, letter in enumerate(string.ascii_uppercase, start=1)}
TRIANGLE_NUMBERS = [
    0.5 * n * (n + 1)
    for n in xrange(int((2 * MAX_TRIANGLE) ** 0.5))
]

with open('words.txt', 'rb') as csvfile:
    count = sum(
        1
        for line in csv.reader(csvfile, delimiter=',', quotechar='"')
        for word in line
        if sum(LETTERS_MAP[char] for char in word) in TRIANGLE_NUMBERS
    )

If you want it to be faster than your original, then you can change MAX_TRIANGLE = 30 * 26 to MAX_TRIANGLE = 15 * 26, bit dodgy IMO.
But this uses less memory, (unless csv loads everything into memory) and is more readable.

To answer your questions:

I would only encorage itertools if you use generators. Which you don't. (Well, you have one that is fed into sum...)
It's not 'Pythonic' as the maths you use isn't. However personally I think a greedy for is better, (hence why my maths is an over estimate).
If you wrapped them in a main that would be good. Wrapping them into functions, is always better than having no functions.
But as this can be done with 3 constants, and 1 generator, I would say one function is fine.
I would say following PEP8 is your best bet for that.


Your code looks fine, however to improve it, you should use a proper test system like pytest or unittest. To demonstrate, here is your code when using pytest, and making the test proper (you don't need to test every specific item:

def flatten(input_array):
    result_array = []
    for element in input_array:
        if isinstance(element, int):
            result_array.append(element)
        elif isinstance(element, list):
            result_array += flatten(element)
    return result_array

def test01():
    results = flatten([1, [2, 3, [4]], 5, [[6]]])
    assert results == [1, 2, 3, 4, 5, 6]

def test02():
    results = flatten([1, [2, 3, [4], []], [], 5, [[], [6]]])
    assert results == [1, 2, 3, 4, 5, 6]

And here are the results:

C:\PycharmProjects\codereview\tests>pytest scratch_14.py
======================== test session starts ========================
platform win32 -- Python 3.7.0, pytest-3.6.2, py-1.5.4, pluggy-0.6.0
rootdir: C:\PycharmProjects\codereview\tests, inifile:
plugins: cov-2.5.1, celery-4.2.0
collected 2 items

scratch_14.py ..                                               [100%]

===================== 2 passed in 0.09 seconds ======================

This is much easier to set up, and less code to write to validate if the solution is correct.
You asked: Is usage of TypeError exception justified?
I don't actually see any code referencing a type error. Did you forget to put it in? Or are you referring to the use of isinstance? If so, that code is fine.
Hope this helps!


Different way of writing multiple click functions

First off, I'd rewrite the HTML to get rid of the javascript href attributes. I'd also use a data-action attribute for the action name. You can still use class to do the styling of course, but style is not behavior, so keep those things separate (i.e. the class might be action-button primary highlighted or something, but the action would still be save):

Edit

    Cancel |
    Save

Code-wise, what each link has in common is that they should preventDefault so the links aren't "followed" when clicked. So at the very least, we'll need this:

$("a[data-action]").on("click", function (event) {
  event.preventDefault();
});

Then of course, there are the actions/behaviors themselves. I'd suggest putting the logic for each in an object, and expanding the generic click handler like so:

var actions = {
  edit:   function (event) { ... },
  cancel: function (event) { ... },
  save:   function (event) { ... }
};

$("a[data-action]").on("click", function (event) {
  var link = $(this),
      action = link.data("action");

  event.preventDefault();

  // If there's an action with the given name, call it
  if( typeof actions[action] === "function" ) {
    actions[action].call(this, event);
  }
});

Now you have a generic click handler, and an easily extensible list of actions. The action functions themselves behave exactly like normal jQuery event handlers (i.e. this will refer to the link clicked, and the first argument will be the event).


I don't do EF, so I am focusing on the code only. But nevertheless maybe this will help you in doing joins with EF: how-to-join-multiple-tables-using-repository-pattern-entity-framework

Based on the naming guidelines input parameters should be named using camelCase casing. So

public List<int> GetUserPrevileges(string UserName)

should be

public List<int> GetUserPrevileges(string userName)

The same is true local varaiables like Privs but in addition you shouldn't shorten your varaiables names because you are reducing the readability of your code. So better use privileges instead of Privs.
This if..else

if (Privs.Any())
{
    return Privs.ToList();
}
else
{
    return null;
}

does not add real value. Why do you want to return null? It would be much easier for the calling code if you return an empty List for the case that there aren't any privileges which match the requirement.

If the IQuerable<T> doesn't contain any items, a call to ToList() will just create an empty list.

If you use the returned List<> to just iterate over the items, you won't even need a check if it is empty.


Generate JSON File from a given YAML file

Simplify File Reading

Java 7 and 8 make I/O a lot simpler. You can simplify your reading of a String from your input file to this:

    private static String readYamlFile(String pathname) throws IOException {
        String lineSeparator = System.getProperty("line.separator");
        List<String> lines = Files.readAllLines(Paths.get(pathname));

        return lines.stream().collect(Collectors.joining(lineSeparator));
    }

Remove the first two Try...Catch block

Your first try...catch blocks catch IOException and then just print out the stack trace. This adds lots of uneccesarry lines for little benefit. If you add throws IOException to public static void main(String args[]) the code becomes clearer, such as:

 public static void main(String[] args) throws IOException {

    //Get File Path
    String sourceFilePath = getSourceFilePath(args);

    //Load File content
    AtomicReference<String> content = new AtomicReference<String>();
    content.set(readYamlFile(sourceFilePath));
    //... and so on

Remove the AtomicReference

The AtomicReference might be useful if the code were multi-threaded. But it's not, the content variable is local to main(). So it just needs to be a String and with the removal of the try..catch above it can be simplified to:

String content = readYamlFile(sourceFilePath);

Don't Use sourceFilePath for your outputFilePath

It's a misleading name, give the output file its own variable. You can also just do it in a single line:

String outputFilePath = sourceFilePath.substring(0, sourceFilePath.length() - 3) +
"json";

Also - this assumes the extension is three characters, so won't work correctly with a .yaml file. You can use a library like apache-commons to do this in a more foolproof way.

String outputFilePath = FilenameUtils.removeExtension(sourceFilePath)
         + ".json";

Simplify File Writing With PrintWriter

Again later versions of Java make dealing with I/O more straight-forward. I've kept the try...catch block so you can dispay the error message. You can also use the try-with resource to auto-close your PrintWriter if anything goes wrong

    try (PrintWriter out = new PrintWriter(outputFilePath)) {
        out.print(jsonStr);
        System.out.println("Success " + sourceFilePath);
    } catch (IOException err) {
        System.out.println("Error writing to file: " + err );
    }


The key is to create a mapping for the edges:

vector<int>
get_pretty_path(const vector<pair<int,int>>& edges, int s, int t) {
    map<int,int> edgeMap = {begin(edges), end(edges)};
    vector<int> path;
    while (s != t) {
        path.push_back(s);
        s = edgeMap[s];
    }
    path.push_back(s);
    return path;
}
int main() {
    const vector<pair<int,int>> edges{{4, 5}, {7, 4}, {2, 3}, {5, 2}};
    const vector<int> path = get_pretty_path(edges, 7, 3);
    for (auto i : path) {
        cout << i << ' ';
    }
    cout << endl;
    return EXIT_SUCCESS;
}

The code assumes using namespace std; which is sometimes frowned
upon but which I think is fine for small examples. The code will bug
out if you can't walk from s to t or if more than one edge starts
at a given node index. If such situations can happen, you have to decide
how to best handle them.


Your name of properties and selectors has consistency, so make a arrays of the parts that change.

I've also added the objects you're mutating and the functions you're calling to other objects so that they can be referenced using a variable.

var am_pms = ["morning", "evening"]
var dirs = ["up", "down"]
var units = ["sec", "min", "hour"]

var am_pm_obj = {
  morning: morning,
  evening: evening
}
var update_funcs = {
  sec: update_sec,
  min: update_min,
  hour: update_hour
}

Then iterate the arrays, patching in the parts. This will cover all the functions you need to create:

am_pms.forEach(function(morn_even) {
  dirs.forEach(function(dir) {
    var offset = dir === "up" ? 1 : -1

    units.forEach(function(s_m_h) {
      $('#' + morn_even + ' .' + dir + ' .' + s_m_h).click(function () {
        am_pm_obj[morn_even][s_m_h] += offset
        update_funcs[s_m_h](am_pm_obj[morn_even])
      })
    })
  })
})

morn_even will be "morning" or "evening"
dir will be "up" or "down"
offset will be 1 or -1, which replaces ++ and --
s_m_h will be "sec", "min" or "hour"

The string variables will be used to build the selector and also to access the objects you mutate (and pass) and the functions you call, which is why the objects and functions needed to be nested in another object.


Bug: Returning a free'd pointer!

After freeing a pointer, do not use it.

        free(head);
        printf("La lista adesso e' vuota. Finito\n");
        // return head;
        return NULL;

  Is there anything else that can be improved?

Rather than 2 or 3 cases to handle an empty list, list of only matches or mixed list, consider a code simplification.

Create a temporary pre-head node and assign its next to the head.  Only 1 case  needed.  This approach is reasonable when node_t is not large.

Some untested code.

node_t *rimuovi(node_t *head, int x) {
  node_t pre_head;
  node_t *p = &pre_head;
  pre_head.next = head;  // Other members are not used.

  while (p->next) {
    if (p->next->val == x) {
      node_t *next = p->next->next;
      free(p->next);
      p->next = next;
    } else {
      p = p->next;
    }
  }
  return pre_head.next;
}


best design pattern for refactoring a set of classes that do calculation base on many parameters

Depending on the rest of your design, having a Parcel know how to calculate it's ownworth is perfectly acceptable behaviour.  That said, the method of calculation is now tied directly to the parcel class itself (what happens if your local government decides that ExpressParcels under a specific weight should be charged like a GeneralParcel - or worse, some other subclass?).

Also, the three dictionaries likely do not belong in the parcel class at all.  I can't imagine that is the only location that type of information is relevant.  Shipping/truck routing procedures also come to mind.  You should see if this is the case, or if the logic/data is indeed unique.

With that stated, here's some possibilities for refactoring in this situation.  You're on the right track with IPriceCalculator; you should modify it to take an IParcel object.

1) Implement IPriceCalculator, store as class (static) variable in IParcel subclasses.

public class GeneralParcel : IParcel {

    private static IPriceCalculator calc = new IPriceCalculator() {

                                               public decimal Calculate(IParcel parcel) {
                                                  return weight * .1;
                                               }
                                           }
}

This allows price calculators to be shared between classes, looked up from a static factory, etc.  However, it doesn't allow easy change of the calculator used for a specific instance of that class (such as, say, applying coupon rules).  And you still have to ask the parcel how much it is.

2)  Implement IPriceCalculator, store as instance variable in IParcel subclasses.

public class GeneralParcel : IParcel {

    private IPriceCalculator calc;

    public decimal Price {
       get {
          return calc.Calculate(this);
       }
    }

    public GeneralParcel(IPriceCalculator calc) {
       this.calc = calc;
    }

}

This allows prices calculators to be shared and swapped out as necessary.  The catch is that an individual parcel can't know all the possible restrictions to make the choice, and a calculator shouldn't know about any of the others.  Parcel still knows how much it is.

3) Implement IPriceCalculator, lookup from stored cache, store result in IParcel subclasses.

public class GeneralParcel : IParcel {

    private decimal price = -1;

    public decimal Price {
       get {
          if (price < 0) {
              price = CalculatorLookup.find(this).calculate(this);
          }
          return price;
        }
    }

    public GeneralParcel() {

    }
}

This of course caches out the information, preventing costly recalcs.  However, getting line-item charges may be dificult, and some of the Lookup rules might be interesting.

Finally, my favorite, and the most flexible;

4) Create (and implement) a series of *PriceCalculation*s, store IPriceCalculation in IParcel subclass instances.

public interface IPriceCalculation {
   public decimal Calculate {get;}

   public IPriceCalculation AddRule(IPriceCalculation calc, IParcel parcel);
}

impl

public struct BaseFeeCalculation {
   private readonly decimal price;

   public decimal Calculate {
      get {
         return price;
      }
   }

   private BaseFeeCalculation (IPriceCalculation calc) {
      if (calc == null) {
          price = 5;
      } else {
          price = 5 + calc.Calculate;
      }
   }

   public IPriceCalculation AddRule(IPriceCalculation calc, IParcel parcel) {
      return new BaseFeeCalculation(calc);
   }

}

I recommend that the calculations be structs, and immutable.  You're probably going to want to add other things to them, like the ability to report their individual price (for line items), and not just the total.  Additionally, while immutablility will help avoid circular references, you may want to add something to check for existing calculations (probably shouldn't have two BaseFees, after all).

Parcel basically stays the same; just add a property that returns the IPriceCalculation used.

When adding rules, try something along the lines of the following:

public struct CalculationBuilder {

   // You'll need to initialize this somehow, obviously.
   private static readonly IList<IPriceCalculation> calculations;

   private CalculationBuilder() {}

   private static IPriceCalculation Build(IParcel parcel) {
      IPriceCalculation calc;

      foreach(IPriceCalculation calcer : calculations) {
         calc = calc.AddRule(calcer, parcel);
      }

      return calc;
   }
 }

This works well for anything short of dependent rules (although can be adapted fairly easily for that).  Usually, the list of calculations can/should be initialized through a DI/Autowiring framework (like Spring).

Oh, and thank you for using decimal, and not double for your price calculation.

... that was a bit long, wasn't it.


Convert zip to gzip and upload to S3 bucket

Looks okay to me in general.  I'd prefer to pass in configuration
separately, parsed from a config file or command line arguments, but if
it's a one-off script it's probably okay this way.

The temporary files aren't deleted as far as I see; consider using the
tempfile module to
allocate and delete temporary files automatically.

Also, while the string to bytes conversion works as is, it's also
brittle when used with large files, e.g. larger then the amount of RAM
available, because it will store the downloaded file in memory.  I'd
suggest passing in gz.write directly instead:

with gzip.open(tmp_dir + tmp_fname, 'wb') as gz:
    ftp.retrbinary('RETR ' + fname, gz.write)

Unless I'm mistaken that should keep the bytes in order; if not you
could still pass in a function that converts only the downloaded chunk
to bytes and calls gz.write on the converted value.

Regarding your questions:

Since there's no extraction of data from the zip file, I don't see how
that matters.  If you were to extract something (a single file) from
the zip file, then yes, gzipping that will of course be okay.  Make
sure that the gzipped files are how you expect them (i.e. a single
compressed text file) and that you don't need the file name or other
attributes in the original zip archive.  At the moment you basically
upload some_file.zip.gz to S3, as in, two compressions nested.
Probably not what you want.
As argued above that's probably not advisable unless you know that the
data fits into memory.  If it does, sure, why not.

Some more remarks:

The zipfile import is unused, as mentioned above.
Prefer
os.path.join
to concatenating strings.
fname/passwd instead of filename/password seems pointless
truncation.
print should be used as a function to be forwards compatible,
i.e. print(e) instead.
The result of tmp_dir + tmp_fname should be reused instead of
writing the same expression twice.


Aggregate prescriptions per drug in a CSV file

Performance

Your performance problem is primarily due to the duplicate_id() function, namely the if id in id_list test.  Testing whether an n-element list contains a particular item is O(n).  If you call that for n rows, then it's O(n2).  That would not be acceptable at all for large files.

Sorting the input would help bring it down to O(n log n).

But really, what you want is a solution with O(n) runtime, based on the hashing principle.  Use dict, set, or similar data structures so that each lookup takes constant time.

Style

Use csv.DictReader to avoid the ugliness of discarding the first row using next(readCSV), as well as the magic column numbers in row[0], row[3], and row[4].

Instead of writing two cases depending on whether an entry already exists in a dictionary, use collections.defaultdict, collections.Counter, dict.get(key, default), or dict.setdefault(key, default).

Suggested solution

from collections import Counter, defaultdict
import csv
import sys

def write_file(output_filename, drugs):
    with open(output_filename, 'w') as f:
        w = csv.DictWriter(f, ('drug_name', 'num_prescriber', 'total_cost'))
        w.writeheader()
        w.writerows({
                'drug_name': drug_name,
                'num_prescriber': len(prescriber_totals),
                'total_cost': sum(prescriber_totals.values()),
            }
            for drug_name, prescriber_totals in drugs.items()
        )

def read_file(input_filename):
    drugs = defaultdict(Counter)
    with open(input_filename) as f:
        for row in csv.DictReader(f):
            drugs[row['drug_name']][row['id']] += float(row['drug_cost'])
    return drugs

def main(input_filename, output_filename):
    write_file(output_filename, read_file(input_filename))

if __name__ == '__main__':
    main(sys.argv[1], sys.argv[2])


Creating a backup of a website directory inside the Web Application Root directory in *nix

Well, to be honest I am not exactly sure what do you want to review :) This isn't a script. It is just standard zip command.

Maybe you should think about this: ${war}/mediawiki_general_backups/${domain}-directory-backup-${date}.zip. It is too long and ugly. Try to add more these things into your script and a readability will be lost.

  I fear I don't have enough testing to ensure that the file was created,

If you want to be sure, that the zip command finished sucessfully, just test it.

#!/usr/bin/env bash

# Any subsequent(*) commands which fail will cause the shell script to exit immediately.
# https://stackoverflow.com/a/2871034/10814755
# Now i should trust the output of zip command.
set -e

#domain=$1 <- It could be useful, isn't?
domain="example.com"

#I omit application root. I thing is safer to keep backups away.
backup_path="/mediawiki_backups/${domain}/"
backup_file="$(date +%F-%T).zip"

# You should trust the zip command, but sure is sure.
# So if backup file exist...
# ...and isn't empty...
if [[ -f ${backup_path}${backup_file} ]] && [[ ! -s ${backup_path}${backup_file} ]]; then
  #... then do something brave :)
else
  echo "Backup failed!"
  exit 1
fi

Regards :)


Since there should be a relation between your importer data reader and data accessor ( importer should know how to use data accessor to access data.) I suggest that you use generics to achieve such coherence.(Since each importer needs its unique data then IDataAccessors can't be replaced by each other thus I don't think that's a good idea to introduce an abstraction for them)

public abstract ImporterDataReader<TAccessor>:IImporterDataReader
{
public abstract bool HasValue(TAccessor accessor);

public abstract string GetValue(TAccessor accessor);
}

Then you can have concrete classes for each type of Accessor:

public class JsonDataReader:ImporterDataReader<JsonDataAccessor>
{
....
}


Making PHP composable and look functional

I propose you a more compact version with code duplication removal:

You can compact all of your call with an apply method, as shown in the following code:

<?php

class Functional
{
    protected $result;
    protected $extra_data;

    public function __construct($array = NULL)
    {
        $this->result = $array;
    }

    protected function apply($fn, $callback, $value)
    {
        $input = (is_null($value)) ? $this->result : $value;
        call_user_func($fn, $input, $callback);
        return $this;
    }

    public function reduce($callback, $i = NULL)
    {
        return $this->apply('array_reduce', $callback, $i);
    }

    public function walk($callback, $i = NULL)
    {
        return $this->apply('array_walk', $callback, $i);
    }

    public function filter($callback, $i = NULL)
    {
        return $this->apply('array_filter', $callback, $i);
    }

    public function result()
    {
        return $this->result;
    }

    public function gather($callback, $key)
    {
        $this->extra[$key] = $callback($this->result);
        return $this;
    }

    public function extra($key)
    {
        return $this->extra[$key];
    }
}


Proper way to insert a record and assign the id to the Entity object

I would use a slightly modified form of your option #2, which, instead of returning a modified version of the object, returns a new instance of that same entity.

There are two main reasons for this. The first is that you can't be entirely sure if the data stored in your DB is exactly that of your original item. The auto-incremented ID field is new, of course, but there could be triggers, constraints or login in stored procedures that might make the data subtly different. It's best to return the newly-stored entity from the DB call and have it fully synced with the DB.
The second reason is immutability. If your logic relies on updating a field in your entity after it's created, it means the ID field is mutable and can be changed elsewhere in your code. Having the class be immutable - unchangeable after instantiation - can make a lot of your logic simpler. You can have two threads operate on the same instance without worrying about race conditions between them, and other benefits beside.

Reason #2 is enough, in my opinion, to always clone your object, whether it's cloning-through-database-query or just cloning the instance before returning it.


Python optimized base64 writer for streamed files

I ended up using bytearray as an input and output buffer. If found that if the output was something that doesn't buffer output (like a socket), then writing 77 bytes at a time would be very slow. Also my original code rounded the read size to be advantageous for base64, but not advantageous for MongoDB. It's better for the read size to match the MongoDB chunk size exactly. So the input is read into a bytearray with the exact size passed in, but then read in smaller base64 size chunks.

def chunked_encode(
        input, output, read_size=DEFAULT_READ_SIZE, write_size=(base64.MAXLINESIZE + 1) * 64):
    """
    Read a file in configurable sized chunks and write to it base64
    encoded to an output file.

    Args:
        input (file): File like object (implements ``read()``).
        output (file): File like object (implements ``write()``).
        read_size (int): How many bytes to read from ``input`` at
            a time. More efficient if in increments of 57.
        write_size (int): How many bytes to write at a time. More efficient
            if in increments of 77.
    """
    # 57 bytes of input will be 76 bytes of base64
    chunk_size = base64.MAXBINSIZE
    base64_line_size = base64.MAXLINESIZE
    # Read size needs to be in increments of chunk size for base64
    # output to be RFC 3548 compliant.
    buffer_read_size = max(chunk_size, read_size - (read_size % chunk_size))

    input.seek(0)

    read_buffer = bytearray()
    write_buffer = bytearray()

    while True:
        # Read from file and store in buffer until we have enough data
        # to meet buffer_read_size
        while input and len(read_buffer) < buffer_read_size:
            s = input.read(read_size)
            if s:
                read_buffer.extend(s)
            else:
                # Nothing left to read
                input = None

        if not len(read_buffer):
            # Nothing in buffer to read, finished
            break

        # Base 64 encode up to buffer_read_size and remove the trailing
        # line break.
        data = memoryview(b2a_base64(read_buffer[:buffer_read_size]))[:-1]
        # Put any unread data back into the buffer
        read_buffer = read_buffer[buffer_read_size:]

        # Read the data in chunks of base64_line_size and append a
        # linebreak
        for pos in xrange(0, len(data), base64_line_size):
            write_buffer.extend(data[pos:pos + base64_line_size])
            write_buffer.extend('\n')

            if len(write_buffer) >= write_size:
                # Flush write buffer
                output.write(write_buffer)
                del write_buffer[:]

    if len(write_buffer):
        output.write(write_buffer)
        del write_buffer[:]

For 10 iterations of a 10 MB file (complete test), this version is up to 5 times faster than standard base64 with large buffer sizes (>969) when reading a file without buffering (like a socket). For small buffer sizes (~100) it is about the same or worse than standard base64.

--- bufsize 4096
standard_base64_encode 5.70770692825 seconds for 10 iterations
original_chunked_encode 2.07641100883 seconds for 10 iterations
latest_chunked_encode  1.44510507584 seconds for 10 iterations
--- bufsize 2048
standard_base64_encode 5.71355605125 seconds for 10 iterations
original_chunked_encode 2.17808198929 seconds for 10 iterations
latest_chunked_encode  1.5746011734 seconds for 10 iterations
--- bufsize 1024
standard_base64_encode 5.7339630127 seconds for 10 iterations
original_chunked_encode 2.35343503952 seconds for 10 iterations
latest_chunked_encode  1.83091807365 seconds for 10 iterations
--- bufsize  969
standard_base64_encode 5.87562203407 seconds for 10 iterations
original_chunked_encode 2.3832950592 seconds for 10 iterations
latest_chunked_encode  1.81391692162 seconds for 10 iterations
--- bufsize  100
standard_base64_encode 5.84305310249 seconds for 10 iterations
original_chunked_encode 6.96859192848 seconds for 10 iterations
latest_chunked_encode  6.85651683807 seconds for 10 iterations
--- bufsize   57
standard_base64_encode 5.72181987762 seconds for 10 iterations
original_chunked_encode 6.98394799232 seconds for 10 iterations
latest_chunked_encode  8.28728795052 seconds for 10 iterations


One thing would be to separate view from functionality (you may even make the business logic invocation asynchronous). You can also make your code cleaner and smaller by creating widgets that support templating and using them.


Generally, the "order" of what you want to pass by looks like this for user-defined classes (ignoring C++11, for the moment):

Pass by const&. This should be the default way of passing
parameters.
Pass by &. If you need to modify the parameter for whatever reason, then
you'll need to pass a reference or pointer to it. However, you should prefer references to pointers (explained below).
Pass by value. Unless you really need to make a copy of the value, there's not much reason to pass by value compared to passing by const&. However, if you're going to be making a copy of the parameter in the function body either way, then you may want to simply pass by value in the first place.

For a good example of when explicit pass by value makes sense, see the copy-and-swap idiom.

However, this is "muddied" by a number of considerations:

Is the class small? Does it only encapsulate a few (simple) data members? For example, something like a std::pair. In this case, it may actually be better to just pass by value.
For fundamental data types (int, char, double, and so on), it's better to simply pass by value. There's no performance benefit to passing by reference, and in fact, it can often be slower.

With move semantics in C++11, this advice changes considerably. We now have an extra possibility: pass by rvalue reference (&&). Passing by value and using std::move then becomes a distinct possibility, especially when we want to be explicit about ownership. You may want to read this Stackoverflow post for a bit more information. Dave Abrahms (one of the principle authors of boost) has also written about this topic.

I haven't mentioned pointers at all until now. Really, the only difference between references and pointers is that references cannot be null. Sticking to references means that you eliminate a whole host of possible bugs to do with dereferencing null pointers. In modern C++, passing by pointer is rare. In fact, it should always be wrapped up in a stack allocated class that deals with the ownership semantics and eliminates the possibility of resource leaks (say, unique_ptr or shared_ptr). These are then passed by one of the ways mentioned above.

Finally, one must use pass by reference or (smart) pointer when using dynamic dispatch - that is, when dealing with inheritance.

This post may be far more than you ever wanted to know, but the answer is effectively "it depends". Deciding how to pass a parameter requires some actual thought, but boils down to some (subset) of the following considerations:

Is it a primitive data type or a class encapsulating only a few primitive datatypes?
Does it encapsulate some resource? If so, after the call, who do I want to own the resource?
Do I need to modify it in the function I'm passing it to? If so, can I rewrite the function in some way so that I don't have to, as this often makes it more difficult to understand the program.
Is inheritance involved? Do I want to be able to pass a derived class where a base class parameter is involved?
Am I going to simply make a copy of the parameter in the function body? If so, perhaps passing by value is the best idea (again, see the copy and swap idiom).
Should I provide overloads so I can have the option of passing a const reference vs passing an rvalue reference?
What will give me the best performance? What is easiest to reason about and maintain?

(Note: I realise this isn't really a code review as such. If this is too off topic, feel free to remove it).


Populating an array with times with half hour interval between them

Naming (more later)

I'd recommend renaming your function to something more specific than getTimes to avoid collisions with other getTimes functions. A more specific name also provides more information on what the function does.

function createHalfHourIntervals() {}

Looser Coupling

Another potential improvement would be to decouple your time formatting from your time interval creating. You could use a sleek functional programming technique to easily add the formatting after you've made the intervals:

function createHalfHourIntervals(from, until){
  // ...
  var time = new Date(from);
  var intervals = []; // more clear name than hours

  for (var i = 0; i <= max; i++) {
    intervals.push(time);
    time.setMinutes(time.getMinutes() + 30);
  }
}

function formatDateHHcolonMM(date) {  // funny name but specific
  var hour = date.getHours();
  var minute = date.getMinutes();
  return doubleZeros(hour) + ":" + doubleZeros(minute);
}

var intervals = createHalfHourIntervals()
  .map(formatDateHHcolonMM); // map will create a new array of formatted intervals from the array returned by createHalfHourIntervals

This separates your formatting logic from your interval creation logic, making your code more reusable and cohesive. If another programmer were less familiar with your code, but wanted to change the formatting of your intervals in someway, that task is made much simpler in this format. Each section of code doing one task, in general, can make the project easier to understand. However, if you don't plan on reusing much of the code in getTimes or working with other developers, sometimes it can just be simpler to keep all the code contained in one single location.

Good Optional Ideas

Although you may not want it now, it's possible that you may want to create arrays of intervals of different lengths in the future. Again, this depends on what you actually need, but 20min intervals or even interval times offset by 5min [7:05, 7:35] can be quite common. You could prepare better for the future change by reworking your function a bit. Your "01/01/2001 ", "*2" and "+30" are what some consider magic numbers, which tend to make the code less readable--sometimes even requiring their own comments. A good variable name oftentimes is better than a comment, so you may want to give these numbers their own variable names, improving clarity while preparing for a future refactoring:

function createHalfHourIntervals(from, until) {
  var arbitraryDay = "01/01/2001 ";
  // ...

  var intervalLength = 30;
  var intervalsPerHour = 60 / intervalLength;
  var milisecsPerHour = 60 * 60 * 1000;
  // are you sure that Math.abs(until - from) is the behavior you want. It seems like preparation to allow the user to put the 'from' and 'until' parameters in the other order.
}

That's pretty much all of your code! Hope you find this helpful, and keep up the good work!


A quick browse through the SWI-Prolog manual finds atomics_to_string/3:

atomics_to_string(+List, +Separator, -String)
Creates a string just like atomics_to_string/2, but inserts Separator between each pair of inputs. For example:
?- atomics_to_string([gnu, "gnat", 1], ', ', A).
A = "gnu, gnat, 1"


1) Which of the moves are actually needed here?

Since there are only two moves, let's have a look at them:

std::unique_ptr<Car> getCar(CarName sa) {
    if (d_carMap.find(sa) == d_carMap.end()) {
        d_carMap[sa] = std::move(createCar(sa)); // (1)
    }
    return std::move(d_carMap[sa]);              // (2)
}

Only the latter is necessary. In (1), the expression createCar(sa) is a prvalue expression and therefore a rvalue. In (2), d_carMap[sa] is a lvalue expression, since operator[] returns a lvalue reference.

Or: if you try to move something that has a name, or if the function returns T&, you need std::move.

  2) What happens to the unique_ptrs, if this factory is used in a multi threading environment?

Undefined behaviour. There's a race condition if one uses std::move on the same unique_ptr twice: the move assignment gets evaluated half-through twice, you end up with two unique_ptr owning the same memory, and two calls to delete. Note that the std::unorded_map isn't thread-safe either.

  3) Is std::unique_ptr(new Taxi()); really the best way of instantiating a smart pointer in C++11 (which doesn't have make_unique)

No. Use make_unique, but make sure to put it in your own namespace, not std. That way, you can simply replace your version with the C++14 one later.

  4) Should the enum be a class, even if I can't use std::hash on it?

That doesn't really make sense. Either you have an enumeration, or you don't. Unless you mean a scoped enumeration (enum class). And yes, scoped enums are usually better. But that depends on 5).

And you can use std::hash on a scoped enum, even in C++11, since you're allowed to specialize std::hash for your type. It's one of the exceptions where you may mingle in the std namespace.

  5) where would you define the enum? In a separate header file? In the same header, outside of the class (but in a namespace)?

The "namespace" part isn't necessary if you use a scoped enum. The place of the enum depends on its use. If it's only used together with this class, it's reasonable to keep it in the same header. If CarName can get used without CarFactory, it's probably a better idea to put it into another unit. But that depends on your use case.

  6) How should the unit test look like? I wrote the following test which actually fails on the indicated line.

And it should fail. You moved the ownership of whatever d_carMap[sa] pointed to outside. However, you didn't remove the actual std::unique_ptr from your map. It's still there, being the same as a nullptr. Therefore, your d_carMap.find() yields an iterator:

// creates a ClownCar, stores it in d_carMap
// in a pair (ClownCar, <some unique_ptr>) and
// immediately moves from the <unique ptr>
// thus d_carMap contains (ClownCar, nullptr)
// after the call.
auto myCar = factory.getCar(ClownCar);

// find(ClownCar) finds (ClownCar, nullptr)
auto myNullCar = factory.getCar(ClownCar);

// whoops

So the problem is that you don't get rid of the nullptr std::unique_ptr in your map. And your test have shown that, so they're actually helping.

Overall I think that you've abstracted the code a little bit too much.  It's intend isn't clear, since the documentation is missing. Also, the indentation isn't uniform, but that's a minor problem.