A recursive_transform for std::vector with various return type This is the follow-up question for A recursive_transform Function For Various Type Nested Iterable With std::variant Implementation in C++ and A get_from_variant function in C++. Thanks to G. Sliepen and Quuxplusone provide detailed review suggestions. However, the existed version of recursive_transform function assumes the return type is always the same as the input type. In other words, it works well with the lambda function like [](double x)->double { return x + 1; } (the type of both input and output are double) or [](int x)->int { return x + 1; } (the type of both input and output are int). In the next step, I want to focus on the case which the return type is different from the input type. For example, [](int x)->std::string { return std::to_string(x); }. Because the origin return type of recursive_transform is specified in T, it can not handle the case which the type of the processed output from the lambda function f is different from T. Let's change type T into auto as below. This auto syntax used here makes type deriving adaptive.
template<class T, class F>
auto recursive_transform(const T& input, const F& f) {
    return f(input);
}

The another part of this recursive_transform is the recursive structure and it is more complex than above. The container has been specified in std::vector here first.
template<class T, class F> requires is_iterable<T>
auto recursive_transform(const T& input, const F& f) {
    typedef typename std::iterator_traits<typename T::iterator>::value_type
        value_type;

    std::vector<decltype(recursive_transform(std::declval<value_type&&>(), f))> output(input.size());
    std::transform(input.begin(), input.end(), output.begin(),
        [f](auto& element)
        {
            return recursive_transform(element, f);
        }
    );
    return output;
}

The test of the above template function recursive_transform.
std::vector<int> test_vector = {
    1, 2, 3
};
auto recursive_transform_result = recursive_transform(
    test_vector,
    [](int x)->std::string { return std::to_string(x); });                          //  For testing

std::cout << "string: " + recursive_transform_result.at(0) << std::endl;            //  recursive_transform_result.at(0) is a std::string

std::vector<decltype(test_vector)> test_vector2 = {
    test_vector, test_vector, test_vector
};

auto recursive_transform_result2 = recursive_transform(
    test_vector2,
    [](int x)->std::string { return std::to_string(x); });                          //  For testing

std::cout << "string: " + recursive_transform_result2.at(0).at(0) << std::endl;     // recursive_transform_result.at(0).at(0) is also a std::string

A Godbolt link is here.
All suggestions are welcome.

Which question it is a follow-up to?
A recursive_transform Function For Various Type Nested Iterable With std::variant Implementation in C++ and
A get_from_variant function in C++

What changes has been made in the code since last question?
In the previous version of recursive_transform function, it works well when the return type is as same as the input type. The main idea in this question is trying to implement a extended version which the return type is different from the input type.

Why a new review is being asked for?
The previous version recursive_transform function assumes the return type is always the same as the input type. I am trying to focus on the case which the return type is different from the input type to make the function more generic. However, I know that I make my algorithms more specialized in std::vector again in this version of code. I have no idea how to implement a more generic recursive_transform function in both various output type and various container type in a simple and smart way. If there is any suggestion or possible idea about this, please let me know.

I'm afraid this is reaching the limits of my knowledge about templates in C++, but I'll try to answer it anyway as best as I can:
Use std::back_inserter() to fill the vectors
Instead of constructing a vector of a given size, just declare an empty vector, but reserve enough capacity, and then use std::back_inserter() to fill it:
std::vector<decltype(recursive_transform(std::declval<value_type&&>(), f))> output;
output.reserve(input.size());

std::transform(input.begin(), input.end(), std::back_inserter(output),
    [f](auto& element)
    {
        return recursive_transform(element, f);
    }
);

However, if you don't use std::vector but a different container type, reserve() and/or std::back_inserter() might not be appropriate.
Determining the container type
So ideally, we don't want to declare a std::vector, but rather the outer container of type T. You can use template template parameters to deconstruct templated types:
template<template<class> class Container, class ValueType, class Function>
requires is_iterable<Container<ValueType>>
auto recursive_transform(const Container<ValueType> &input, const Function &f)
{
    // You want to be able to write this:
    using TransformedValueType = decltype(recursive_transform(*input.begin(), f));
    Container<TransformedValueType> output;
    ...
}

Unfortunately, that doesn't work, at least not with Clang, because a std::vector actually has two template parameters, and other containers might have more or less template parameters. So the solution to that problem is to declare ValueType as a template parameter pack:
template<template<class...> class Container, class Function, class Ts...>
requires is_iterable<Container<Ts...>>
auto recursive_transform(const Container<Ts...> &input, const Function &f)
{
    using TransformedValueType = decltype(recursive_transform(*input.begin(), f));
    Container<TransformedValueType> output;
    ...
}

Although of course this doesn't forward the second template parameter. Putting everything so far together:
template<typename T>
concept is_iterable = requires(T x)
{
    *std::begin(x);
    std::end(x);
};

template<class T, class Function>
auto recursive_transform(const T &input, const Function &f)
{
    return f(input);
}

template<template<class...> class Container, class Function, class... Ts>
requires is_iterable<Container<Ts...>>
auto recursive_transform(const Container<Ts...> &input, const Function &f)
{
    using TransformedValueType = decltype(recursive_transform(*input.begin(), f));
    Container<TransformedValueType> output;

    std::transform(std::begin(input), std::end(input), std::back_inserter(output),
        [&](auto &element)
        {
            return recursive_transform(element, f);
        }
    );

    return output;
}


Calculate possible balances in piggy-bank by weight I have solved Piggy-Bank problem on SPOJ using dynamic programming.

The question asks you to get the minimum value that is possible with given weight (\$w\$) and one or more coins (\$n\$) with given value, \$value[j]\$ and weight, \$weight[j]\$.

I am using following recurrence relation (zero indexed) to solve the problem:

$$dp[i] = \min\left \{ val[j]  + dp[i - weight[j]]\right \}  \forall j \in \left [ 0..n-1  \right ] \&  i - weight[j]  \geq 0$$

where \$dp[i]\$ is the minimum monetary value in a piggy bank with weight \$i\$.

\$dp[w]\$ gives the minimum values possible with given coins for given weight.

But it seems like the question is more quickly solved using a knapsack approach. Can anyone kindly tell why so and why is my approach inefficient?

This code is giving correct answer for all the sample cases:

#include <iostream>
#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <string>
#include <climits>
#define out(i) printf("%d",i)
#define out2(i,j) printf("%d %d",i,j)
#define nl printf("\n")
#define in(i) scanf("%d",&i)
#define in2(i,j) scanf("%d %d",&i,&j)
#define sn(i) scanf("%s",i)
#define inf 0x3f3f3f3f
using namespace std;
int val[500], weight[500], dp[10001],n;

int getAnswer(int w)
{
    if(dp[w] != inf) return dp[w];

    for(int i = 0; i < n; i++)
    {
        if(w - weight[i] >= 0)
        dp[w] = min(dp[w],val[i] + getAnswer(w - weight[i]));
    }

    return dp[w];
}
int main()
{
    // your code goes here
    int t;
    in(t);

    while(t--)
    {
        int e,f,w,ans;
        in2(e,f);
        w = f - e;
        for(int i = 0; i <= w; i++)
            dp[i] = inf;
        in(n);
        dp[0] = 0;
        for(int i = 0; i < n; i++)
            in2(val[i], weight[i]);

        ans = getAnswer(w);
        if(ans != inf)
            printf("The minimum amount of money in the piggy-bank is %d.\n",ans);
        else
            printf("This is impossible.\n");
    }
    return 0;
}

Well, I don't have comments on the algorithm, but if you don't mind, I'd like to recommend a few improvements in the code. Just because it is a programming challenge, doesn't mean we shouldn't write good and clean code.

First major issue with the code is the excessive use of macro constants and function-like marcos. This is a deprecated practice in C++ since the language offers inline functions and typed constants. So all those shady 2-3 letter function-macros should really be actual functions.

Also, give them better names. It is pretty hard to tell just by looking at the code what a call to in() is. There's also an in2(), how's that different from in()? The only difference seems to be that the latter takes two parameters. Well, if you use functions, you don't need two names, since a single name can be overloaded.

#define inf 0x3f3f3f3f

What's the meaning of the constant (which should really be a const int)? It is not the largest value you can fit in a 32bit integer. So what's the story behind that? If you're not going to encode that info in the name, at least provide a comment with the reasons behind that choice. If you are just looking for a very large number, than perhaps use std::numeric_limits<int>::max().

int val[500], weight[500], dp[10001],n;

No need to make these variables global if they can be declared in the program stack and be passed as functions parameters, though the arrays are big. So perhaps they should be std::vectors.

int main()
{
    // your code goes here

Yep, I'd guess so ;)


Partial anagram generator in VBA Excel I'm trying to make a VBA code to arrange k-element subset of an n-set into some sequence. In other word, I'm trying to list all of k-permutations of n member set. For example, let's try to list all of 2-permutations of set {A,B,C} where each characters are located in the cells of Range("A1:C1"). Here are all the permutations:

{A,B}       {A,C}       {B,A}       {B,C}       {C,A}       {C,B}

The following code to implement the above task works fine if there's no duplicate in each of characters of data input:

Sub Permutation()
Dim Data_Input As Variant, Permutation_Output As Variant
Dim Output_Row As Long, Last_Column As Long

Rows("2:" & Rows.Count).Clear
Last_Column = Cells(1, Columns.Count).End(xlToLeft).Column
Data_Input = Application.Transpose(Application.Transpose(Range("A1", Cells(1, Last_Column))))

k = InputBox("Input the value of k for P(" _
    & UBound(Data_Input) & " , k) where k is an integer between 2 and " _
    & UBound(Data_Input) & " inclusive.", "Permutation", 1)

If k >= 2 And k <= UBound(Data_Input) Then
    Output_Row = 2
    ReDim Permutation_Output(1 To k)
    Call Permutation_Generator(Data_Input, Permutation_Output, Output_Row, 1)
Else
    MsgBox "The input [" & k & "] is invalid. The input must be an integer between 2 and " _
    & UBound(Data_Input) & " inclusive."
End If

End Sub

Function Permutation_Generator(Data_Input As Variant, Permutation_Output As Variant, _
                               Output_Row As Long, Output_Index As Integer)
Dim i As Long, j As Long, P As Boolean

For i = 1 To UBound(Data_Input)
    P = True
    For j = 1 To Output_Index - 1
        If Permutation_Output(j) = Data_Input(i) Then
            P = False
            Exit For
        End If
    Next j
    If P Then
        Permutation_Output(Output_Index) = Data_Input(i)
        If Output_Index = k Then
            Output_Row = Output_Row + 1
            Range("A" & Output_Row).Resize(, k) = Permutation_Output
        Else
            Call Permutation_Generator(Data_Input, Permutation_Output, Output_Row, Output_Index + 1)
        End If
    End If
Next i
End Function

I am hoping I can get some help to improve its performance and to better its algorithm in order to make a good partial anagram generator, i.e. it must be able to work if there are the duplicate characters. For example, let's test to list all of characters in my name: ANA. The output should be ANA, AAN, and NAA, but my code returns nothing. For 2-permutations of my name should be AN, AA, and NA yet my code returns AN, NA, AN, and NA. I have a feeling the culprit is the following statements:

        If Permutation_Output(j) = Data_Input(i) Then
            P = False
            Exit For
        End If

though I'm not so sure. I'd be eternally grateful if someone here could help me.

The following code works fine and faster than the previous one to obtain all of k-permutations provided there's no duplicate data in the input. I use an array as the output in the loop instead of the range of cells. It's usually considered to be faster than working on the range of cells. This code also gives two different choices of output. See the comment for the clarity.

Dim k As Long, Permutation_Table
Sub Permutation()
Dim Data_Input, Permutation_Output
Dim Output_Row As Long, Last_Column As Long, Array_Row As Long

Rows("2:" & Rows.Count).Clear
Last_Column = Cells(1, Columns.Count).End(xlToLeft).Column
Data_Input = Application.Transpose(Application.Transpose(Range("A1", Cells(1, Last_Column))))

k = InputBox("Input the value of k for P(" _
    & UBound(Data_Input) & " , k) where k is an integer between 2 and " _
    & UBound(Data_Input) & " inclusive.", "Permutation", 1)

Array_Row = WorksheetFunction.Fact(k) * WorksheetFunction.Combin(UBound(Data_Input), k)

ReDim Permutation_Table(1 To Array_Row, 1 To k)

If k >= 2 And k <= UBound(Data_Input) Then
    ReDim Permutation_Output(1 To k)
    Call Permutation_Generator(Data_Input, Permutation_Output, Output_Row, 1)
Else
    MsgBox "The input [" & k & "] is invalid. The input must be an integer between 2 and " _
    & UBound(Data_Input) & " inclusive."
End If
Range("A3").Resize(Array_Row, k) = Permutation_Table    'Use this line if UBound(Data_Input) < 10
End Sub

Function Permutation_Generator(Data_Input As Variant, Permutation_Output As Variant, _
                               Output_Row As Long, Output_Index As Integer)
Dim i As Long, j As Long, P As Boolean

For i = 1 To UBound(Data_Input)
    P = True
    For j = 1 To Output_Index - 1
        If Permutation_Output(j) = Data_Input(i) Then
            P = False
            Exit For
        End If
    Next j
    If P Then
        Permutation_Output(Output_Index) = Data_Input(i)
        If Output_Index = k Then
            Output_Row = Output_Row + 1
            For n = 1 To k
                Permutation_Table(Output_Row, n) = Permutation_Output(n)
            Next n
            Debug.Print Join(Permutation_Output, ",")    'Optional, use this line as the output if UBound(Data_Input) > 9
        Else
            Call Permutation_Generator(Data_Input, Permutation_Output, Output_Row, Output_Index + 1)
        End If
    End If
Next i
End Function


Java Stack Implementation with Generics I've written an implementation of a Stack data structure using a linked list.

Stack.java

import java.util.EmptyStackException;

public class Stack<T> {

    private class Node {

        private T data;
        private Node prev, next;

        public Node() {
        }

        public Node(T data) {
            this.data = data;
        }
    }

    private Node top;

    public Stack() {
        top = null;
    }

    public boolean empty() {
        return top == null;
    }

    public T peek() {
        if (top == null) {
            throw new EmptyStackException();
        }
        return top.data;
    }

    public T pop() {
        if (top == null) {
            throw new EmptyStackException();
        }
        T value = top.data;
        top = top.prev;
        return value;
    }

    public T push(T data) {
        Node temp = new Node(data);
        if (top == null) {
            top = temp;
        } else {
            top.next = temp;
            temp.prev = top;
            top = temp;
        }
        return temp.data;
    }
}

public T push(T data) {
    Node temp = new Node(data);
    if (top == null) {
        top = temp;
    } else {
        top.next = temp;
        temp.prev = top;
        top = temp;
    }
    return temp.data;
}

This can be simplified; since both cases have top = temp at the end, you can move it out of the if statement:

public T push(T data) {
    Node temp = new Node(data);
    if (top == null) {
    } else {
        top.next = temp;
        temp.prev = top;
    }
    top = temp;
    return temp.data;
}

And as a result of that, there's only 1 case you're interested in:

public T push(T data) {
    Node temp = new Node(data);
    if (top != null) {
        top.next = temp;
        temp.prev = top;
    }
    top = temp;
    return temp.data;
}

public T peek() {
    if (top == null) {
        throw new EmptyStackException();
    }
    return top.data;
}

public T pop() {
    if (top == null) {
        throw new EmptyStackException();
    }
    T value = top.data;
    top = top.prev;
    return value;
}

I'd extract the throwing to a separate function, like so:

public void throwIfEmpty(){
    if(empty()){
        throw new EmptyStackException();
    }
}

and use like this:

public T pop() {
    throwIfEmpty();

    T value = top.data;
    top = top.prev;
    return value;
}

Keep the blank line in there, it's for showing the difference between your guard clauses and the code that does the work.

        top.next = temp;

This is the only part in your code where you set the next attribute. Since it's not read, it can go; you have no need for a doubly-linked list and can just use a singly linked list. It makes sense, because all you need to access is the top node; the other nodes aren't important.

We can further improve things:

public T push(T data) {
    Node temp = new Node(data);
    if (top != null) {
        top.next = temp;
        temp.prev = top;
    }
    top = temp;
    return temp.data;
}

Now that we decided to remove the next setting,

public T push(T data) {
    Node temp = new Node(data);
    if (top != null) {
        temp.prev = top;
    }
    top = temp;
    return temp.data;
}

we really don't need that null check anymore. Either it is null, in which case you're going to skip setting temp.prev to the null it already is, or it is not null, in which case you want to set the value.

In both cases, simply setting the value gives the wanted outcome: the code does not crash and the top node's prev pointer points to the lower node if it exists, else to null.

So remove the if statement.

public T push(T data) {
    Node temp = new Node(data);
    temp.prev = top;
    top = temp;
    return temp.data;
}

Final result:

import java.util.EmptyStackException;

public class Stack<T> {

    private class Node {

        private T data;
        private Node prev;

        public Node() {
        }

        public Node(T data) {
            this.data = data;
        }
    }

    private Node top;

    public Stack() {
        top = null;
    }

    public void throwIfEmpty(){
        if(empty()){
            throw new EmptyStackException();
        }
    }

    public boolean empty() {
        return top == null;
    }

    public T peek() {
        throwIfEmpty();
        return top.data;
    }

    public T pop() {
        throwIfEmpty();

        T value = top.data;
        top = top.prev;
        return value;
    }

    public T push(T data) {
        Node temp = new Node(data);
        temp.prev = top;
        top = temp;
        return temp.data;
    }
}


Accessing parent scope from directive This is how I can access the exact scope value, otherwise it will be $scope.type of local scope.

I am accessing this from the template in NSPopover. NSPopover directive has scope: true.

It's also inside the Angular bootstrap module.

<label ng-class="{selected: $parent.$parent.$parent.type == current.type}" >
    <input type="radio" name="type" ng-value="current.type" ng-change="chooseOption()" ng-model="$parent.$parent.$parent.type"/>

Using $parent is an anti-pattern and sign of bad architecture. What if you code changes together with your parent chain - it will instantly break your code.

Also, have you thought about how you are going to test it?

To have a clean readable code, you want someone looking at it immediately see the meaning. What is the meaning of this object? -

ng-model = "$parent.$parent.$parent.type"

Would you not rather see

ng-model = "selectedItem"

or something more concrete like

ng-model = "myCar"

Naming something "current" makes me wonder what it is. From your code, I have no idea, so it forces me to spend time on looking around and getting clues, for what should be made clear right up front.


Is code optimization for branch predictor necessary now? I would like to share a part of some code written several years ago (I have changed from mysql to mysqli); this code works perfectly. I am new to my company, and our senior programmers wrote some extra lines to optimize branch predictions. My question is: is it necessary to follow this code? Does it improve performance?

We are hosting our files on Linux VPS CentOS 64Bit, 4GM RAM

 $gq = array();   //to hold global ques
 $gq = array_merge($m1_array,$m2_array,$m3_array,$m4_array,$m5_array,$mp5_array) ;
        unset ($m1_array,$m2_array,$m3_array,$m4_array,$m5_array,$mp5_array);
        shuffle($gq);
    // collect answers
    //preparing list of question ids to get the answer
    $qnolist ='';
    foreach ($gq as $qm)
            {$qnolist = $qnolist.','.$qm['qid'];}
    $qnolist=substr_replace($qnolist ,'',0,1);
    //sorted list qry always good performance (branch predictor)
    $qnoarray =explode(",",$qnolist);
    sort($qnoarray);
    $qnolist=implode(",",$qnoarray);
    $sql = "SELECT some fields where `qid` in ({$qnolist}) order by `qid`,`aid`;";
    $result = mysqli_query ($dbcon, $sql) or die("Server connection error...");
      $ga = array();
        while ($row = mysqli_fetch_assoc($result)) {
              array_push($ga, $row);
              }
        mysqli_free_result($result);

@rolfl's answer covers the major points very well.  In addition, I would recommend that you change the comment, removing the phrase "branch predictor."  A branch predictor is a part of the CPU which attempts to predict the result of a branch so that the CPU can speculatively execute the predicted branch.  A programmer can write code that optimizes branch prediction in assembly, or C, or other compiled languages for which the programmer can reliably predict the machine code that the compiler will emit.  In an interpreted language such as PHP or SQL, there is so much code between your source and the machine that optimizing for the branch predictor at the level of your PHP or SQL source no longer makes any sense.


PHP page to edit a user account information and avatar I wanted to ask your opinion about my code created for acccount.php page for editing the data of each logged-in user.

I wanted to have more of an opinion on your right syntax, though maybe it could be improved.

    require('includes/config.php');

//if not logged in redirect to login page
if(!$user->is_logged_in()) {
    redirect('login.php');
}

//define page title
$title = 'Account';

//include header template
require('layout/header.php');

//submit post
if (isset($_POST['location'], $_POST['bio'], $_POST['relationship'], $_POST['interested'], $_POST['work'], $_SESSION['username'])) {
    try {
    //query update
    $stmt = $db->prepare("UPDATE members SET location = ?, bio = ?, relationship = ?, interested = ?, work = ? WHERE username = ?");
    $stmt->execute([$_POST['location'], $_POST['bio'], $_POST['relationship'], $_POST['interested'], $_POST['work'], $_SESSION['username']]);
      $message = $lang['update-profile-success']; //success message
    } catch(PDOException $e) {
    $message = $lang['update-profile-error']; //error message
    }
  }

// ******************  AVATAR  *************************
//choosing the avatar:
    if(isset($_POST['upload']))
    {

        $imgFile = $_FILES['avatar']['name'];
        $tmp_dir = $_FILES['avatar']['tmp_name'];
        $imgSize = $_FILES['avatar']['size'];

        if(empty($imgFile)){
            $message = "Please Select Image File.";
        }
        else
        {
            $upload_dir = 'avatar/'; // upload directory

            $imgExt = strtolower(pathinfo($imgFile,PATHINFO_EXTENSION)); // get image extension

            // valid image extensions
            $valid_extensions = array('jpeg', 'jpg', 'png', 'gif'); // valid extensions

            // rename uploading image
            $userpic = rand(1000,1000000).".".$imgExt;

            // allow valid image file formats
            if(in_array($imgExt, $valid_extensions)){
                // Check file size '5MB'
                if($imgSize < 5000000)              {
                    move_uploaded_file($tmp_dir,$upload_dir.$userpic);
                }
                else{
                    $message = "Sorry, your file is too large.";
                }
            }
            else{
                $message = "Sorry, only JPG, JPEG, PNG & GIF files are allowed.";
            }
        }

        // if no error occured, continue ....
        if(!isset($message))
        {

        $edit = $db->prepare("UPDATE members SET user_avatar = :avatar WHERE username ='" . $_SESSION['username'] . "'");
        $edit->bindParam(':avatar', $userpic);
        $row['user_avatar'] = $userpic;

            if($edit->execute())
            {
                $message = "Avatar updated...";
                header("refresh:5;account.php"); // redirects image view page after 5 seconds.
            }
            else
            {
                $message = "Error...";
            }
        }
    }

//run data form
$sql = $db->prepare("SELECT * FROM members WHERE username = ?");
$sql->execute([$_SESSION['username']]);
$row = $sql->fetch();

Security

Possibly Broken Authentication

if(!$user->is_logged_in()) {
    redirect('login.php');
}

[do important stuff]

Make sure that redirect dies. The header redirect you do use doesn't, and if the one of redirect doesn't either, an attacker could easily bypass this check, as a client does not have to follow a header redirect if they do not want to.

Possibly SQL Injection

You mostly use prepared statements, which is good. It's a bit surprising that you do not always use them though:

$edit = $db->prepare("UPDATE members SET user_avatar = :avatar WHERE username ='" . $_SESSION['username'] . "'");

Depening on your - current or future - limitations on username, this is open to SQL injection. You really should always use prepared statements, even if you think the data may be safe. If you need to think about this each time, you will eventually make a mistake.

Misc

your indentation is off, making your code harder to read.
you have quite a lot of comments which only describe what the code does. Most of them can simply be removed, others seem to be there for structuring your code (AVATAR, choosing the avatar, etc). Instead, add more classes/files and functions. Eg you could have a Avatar class, which has a function uploadAvatar (uploadAvatar would likely use a more generic uploadImage or uploadFile function stored in a more generic class).
your code is quite deeply nested, making it hard to read. If you follow the advice of adding more functions, you will already greatly reduce the nesting. Additionally, return early. So if you have checks at the beginning of a function, don't do if (cond) [do the complex thing], but instead if (!cond) return; [newline] [do the complex thing] (instead of just returning, you could return true/false, an error message, throw an exception, etc.
Error... is not a very informative error message.


Correct Implementation of Custom UIView into ViewController Programatically I am familiar with using storyboards to construct UI elements in Xcode, but I am only beginning to learn how to make UI programmatically (without a storyboard).

I wanted to know if this is the correct way to implement a UIView into an existing ViewController class.

DetailView (Custom UIView class):

.h file (header)

@interface DetailView : UIView

@property (strong, nonatomic) UIImageView *imageView;
@property (strong, nonatomic) UIImageView *ratingView;
@property (strong, nonatomic) UILabel *tagLabel;
@property (strong, nonatomic) UILabel *plotLabel;

@end

.m file (implementation)

- (instancetype)init {

if(self = [super init]){
    // Creating the large movie image
    _imageView = [[UIImageView alloc] initWithFrame:CGRectMake(0.0, 64.0, 375.0f, 200.0f)];

    // Creating the rating image
    _ratingView = [[UIImageView alloc] init];

    // Creating the tag Label
    _tagLabel = [[UILabel alloc] initWithFrame:CGRectMake(38, 274, 355, 200)];
    self.tagLabel.font =  [UIFont fontWithName:@"HelveticaNeue" size:12];

    // Creating the plot label
    _plotLabel = [[UILabel alloc] initWithFrame:CGRectMake(10, 299, 355, 200)];
    self.plotLabel.font = [UIFont fontWithName:@"HelveticaNeue" size:14];

    [self addSubview:self.imageView];
    [self addSubview:self.ratingView];
    [self addSubview:self.tagLabel];
    [self addSubview:self.plotLabel];
}

return self;
}

View Controller:

Commented code is another way I attempted to implement the DetailView

- (void)viewDidLoad {
[super viewDidLoad];
self.detailView = [[DetailView alloc] init];

self.title = self.movie.title;
self.view.backgroundColor = [UIColor whiteColor];

self.detailView.imageView.image =
[UIImage imageWithData:[NSData dataWithContentsOfURL:[NSURL URLWithString:self.movie.fanArt]]];
//[self.view addSubview:self.detailView.imageView];

[self setRatingImage:self.movie.rating];
//[self.view addSubview:self.detailView.ratingView];

self.detailView.tagLabel.numberOfLines = 0;
self.detailView.tagLabel.text = self.movie.tagline;
[self.detailView.tagLabel sizeToFit];
//[self.view addSubview:self.detailView.tagLabel];

self.detailView.plotLabel.numberOfLines = 0;
self.detailView.plotLabel.text = self.movie.plot;
[self.detailView.plotLabel sizeToFit];
//[self.view addSubview:self.detailView.plotLabel];

[self.view addSubview:self.detailView];
}

A couple of thoughts:

It is curious to do some of the configuration of the subview's within your custom class (e.g. the frames) and other portions (e.g. numberOfLines and sizeToFit) in the view controller. If I were going to have this UIView subclass, then I'd encapsulate all the presentation details within that class. As it is, if the UI isn't quite right, you'll never quite know which class you have to take a look at.
It is curious to use numberOfLines of zero, but then to use hard-coded frame sizes. (Sure, you sizeToFit, but don't adjust the origin.) If one label changes size, wouldn't you want to move the other controls accordingly? You might contemplate specifying preferredMaxLayoutWidth, too.
In this day and age of differing device dimensions, it is curious to see hard-coded frame sizes at all. I'd probably implement auto layout unless you had some compelling reason not to. And even if I didn't use auto layout, I'd still specify the autosizing masks.
I generally give my subviews weak references because the process of doing addSubview already maintains a strong reference, and it's not clear your really need two strong references. (Clearly, you have to temporarily use a local variable when instantiating the label, if you go this route.)

Clearly you're not contemplating it at this point, but let's say you ever wanted to do [self.tagLabel removeFromParentView]. Would you also want to have to remember to do self.tagLabel = nil as well?

It's not a big deal, but I tend to follow the NIB/storyboard model that I don't generally maintain strong references to subviews: The only strong reference I'd maintain would be to the root view.
You're never setting the frame for your ratingView. I might confirm the clipping and content mode, too.
If you're going to have this UIView subclass, it doesn't feel like the view controller should be playing around with which image to show in the rating view. The view controller should probably just tell the UIView subclass what the rating is, and that subclass should take care of the presentation of the appropriate rating image.
Rather than using fixed fonts, if this was an iOS 7+ target, I'd consider using Dynamic Type.
If [NSURL URLWithString:self.movie.fanArt] is in the local file system, you can get away with doing that main thread, but if it's a remote URL, then definite retrieve that image asynchronously.
In terms of having custom UIView subclass at all, I'm not sure there's enough utility in this subview to warrant a subclass. Perhaps, once you start incorporating the above points, there might be enough substance to make this worth while, but I'm not sure your existing code sample justifies it.


Flask Database Model Design I am developing my first website in Flask using PostgreSQL and I want my database to have professors from all universities and courses in the USA.

As I am not very familiar with databases I have experienced some issues with my database model design.

Basically, I want to have a Professor, College and  Course models.

1) A professor has:

An id (primary key)
A name
A college (foreign key)
A course (foreign key)

2) A college has:

An id (primary key)
A name (e.g. Harvard University)
Many professors

3) A course in my model is not a college-specific course (e.g. Harvard CS) but a subject which can be identical throughout many colleges, e.g. Biology. It therefore has:

An id (primary key)
A name (e.g. Biology)
Many professors

My end goal is just to have a list of all professors and people can search through them by filtering according to a subject, college, or both.

As you can see I haven't implemented a relationship between colleges and courses.

This is my code:

class Professor(db.Model):
  id = db.Column(db.Integer, primary_key=True)
  name = db.Column(db.String(80), unique=False)
  college_id = db.Column(db.Integer, db.ForeignKey('college.id'))
  course_id = db.Column(db.Integer, db.ForeignKey('course.id'))

class College(db.Model):
  id = db.Column(db.Integer, primary_key=True)
  name = db.Column(db.String(120), unique=True)
  professors = db.relationship('Professor', backref='college', lazy='dynamic')

class Course(db.Model):
  id = db.Column(db.Integer, primary_key=True)
  name = db.Column(db.String(120), unique=True)
  professors = db.relationship('Professor', backref='course', lazy='dynamic')

I was wondering if this is a good database design. Am I going to be able to do what I'm aiming for with the DB models staying the same?

I think at least the relationship between Professor and Course is many to many.

courses_professors = db.Table(
    db.Column(db.Integer(), db.ForeignKey("course.id"),
    db.Column(db.Integer(), db.ForeignKey("professor.id"))

class Course(db.Model):
    id = db.Column(db.Integer(), primary_key=1)
    name = db.Column(db.String(128), unique=1)

class Professor(db.Model):
    id = db.Column(db.Integer(), primary_key=1)
    name = db.Column(db.String(64))
    courses = db.relationship("Course", secondary=courses_professor, backref=db.backref("professors")

If the relationship between Professor and College is also many to many, you can also define a association table between them as preceding codes.


Comparing a selected attribute of a color with a given value I need to call different functions depending on which enum value is used as parameter.
it's working as it should do, but I wanted to ask if there's a proper way to do this without lacking a good performance?

public enum Comparator
{
   Greater=1,Equal=0,Lower=-1
}
public enum ComparableValues
{
    R,G,B,Yellow,Violett,Turquoise,Sum,Average,Intensity
}
public override bool Process(Color cl,Comparator Comp,ComparableValues SelectedValue,float Value)
{
    if (SelectedValue == ComparableValues.R)
    {
        return Value.CompareTo(cl.R) ==(int) Comp;
    }
    if (SelectedValue == ComparableValues.G)
    {
        return Value.CompareTo(cl.G) == (int)Comp;
    }
    if (SelectedValue == ComparableValues.B)
    {
        return Value.CompareTo(cl.B) == (int)Comp;
    }
    if (SelectedValue == ComparableValues.Average)
    {
        return Value.CompareTo(cl.Average()) == (int)Comp;
    }
    return Value.CompareTo(cl.Sum()) == (int)Comp;
}

I know that it could be done by a Function table, like this:

Dictionary<Tuple<ComparableValues, Comparator>, Func<Color, float, bool>>

or by a Dictionary-in-Dictionary or a Dictionary with these ints enums merged to an int...

But obviously all these variants don't feel like they would be proper C#-state of the art stuff.

You can greatly improve it by staring with proper names.

The first enum is actaully a comparison type so name it like so:

public enum ComparisonType
{
    LowerThen = -1,
    Equal = 0,
    GreaterThen = 1,
}

The second enum is a list of color properties but you are allowed to use only one property at a time so your enum should not have a plural name. Let it be ColorProperty:

public enum ColorProperty
{
    R, G, B, Yellow, Violett, Turquoise, Sum, Average, Intensity
}

Calling your methods simply Process isn't good because it doesn't tell the user anything about what it does. Virtually every method processes something so the current name is too general. Think for a while... what does it do? It compares properties of a color with some value so how about IsProperty? Ok, but where is the color part? It's not necessary because will make it an extension of the color so the usage will be clear when used in context.

After applying these changes your method should look like this:

public static bool IsProperty(this Color color, ColorProperty colorProperty, ComparisonType comparisonType, float value)
{
    var colorPropertyGetters = new Dictionary<ColorProperty, Func<Color, int>>
    {
        [Color.R] = c => c.R,
        // ...
        [Color.Average] = c => c.Average(),
    };

    return value.CompareTo(colorPropertyGetters[colorProperty](color)) == (int)comparisonType;
}

Notice that I've also removed all the ifs and replaced them with a dictionary of Funcs that get the property to be comapared.

Now when you use it with a color there is no doubt what is going on here:

var result = color.IsProperty(ColorProperty.Yellow, ComparisonType.Equal, 5);

or

var result = color.IsProperty(ColorProperty.Average, ComparisonType.GreaterThen, 90);

Adjusting the arguments to be in a very specific order and the names for the enum values make the method call read very natural and similar to normal speech.


Is there a better way to implement this Huffman Algorithm in Python? I have written this implementation of Huffman coding. Please suggest ways to make this code better, i.e. more Pythonic.

from collections import Counter

#####################################################################

class Node(object):
  def __init__(self, pairs, frequency):
    self.pairs = pairs
    self.frequency = frequency

  def __repr__(self):
    return repr(self.pairs) + ", " + repr(self.frequency)

  def merge(self, other):
    total_frequency = self.frequency + other.frequency
    for p in self.pairs:
      p[1] = "1" + p[1]
    for p in other.pairs:
      p[1] = "0" + p[1]
    new_pairs = self.pairs + other.pairs
    return Node(new_pairs, total_frequency)

#####################################################################

def huffman_codes(s):
  frequency_table = Counter(s)
  initial_table = []
  for k, v in frequency_table.items():
    initial_table.append([k, v])
  initial_table = sorted(initial_table, key = lambda p : p[1])
  # print(initial_table)
  table = []
  for entry in initial_table:
    ch = entry[0]
    freq = entry[1]
    table.append(Node([[ch, ""]], freq))
  # print(table)
  while len(table) > 1:
    first_node = table.pop(0)
    second_node = table.pop(0)
    new_node = first_node.merge(second_node)
    table.append(new_node)
    table = sorted(table, key = lambda n : n.frequency)
    # print(table)
  return dict(map(lambda p: (p[0], p[1]), table[0].pairs))

#####################################################################

print(huffman_codes('yashaswita'))

Thanks

Really does belong on codereview.

Use four space indentation. This is the Python standard, and will make any code more "Pythonic". Edit: You're following PEP-8 in all other respects as far as I can tell, so if you like two space indentation, it's not a big deal. Your code is very easy to read.
You don't need to sort every time; you don't even need a fully sorted list. Python has a datatype for this -- it's called heapq. Use it's heappush and heappop methods, so you don't have to sort every time. You can actually write a single line loop to do the tree building. Read what that page says about priority queues -- it's what you're using.
You don't need anything like

return dict(map(lambda p: (p[0], p[1]), table[0].pairs))

just

return dict(table[0].pairs)

does exactly the same thing.
If you really want to minimize lines, everything before the while can be written as one line:

table = sorted((Node([[p[0], ""]], p[1]) for p in Counter(s).iteritems()), key = lambda n : n.frequency)


Validating username fields and showing CAPTCHA I'm new to coding, and especially to OOP PHP. I'll highly appreciate if you expert programmers could review following code and give me your opinion to make it better. Also, how you see this code, is it too horrible and ugly?

<?php
include_once("config.inc.php");

class user{

    var $mysqli;
    var $username;
    var $userid;

     function __construct($mysqli) {
         session_start();
         $this->mysqli = $mysqli;
    }

    function valid_length($input){
        if ((strlen($input) > 4 ) AND (strlen ($input) < 16) ){
            return true;
        } else {
            return false;
        }
    }

    function valid_email($email){
        if (preg_match("/^[0-9a-z]+(([\.\-_])[0-9a-z]+)*@[0-9a-z]+(([\.\-])[0-9a-z-]+)*\.[a-z]{2,4}$/i" , $email)) {
            return true;
        } else {
            return false;
        }
    }

    function alpha_numeric($input){
        if (eregi("^([0-9a-z])+$", $input)){
            return true;
        }else{
            return false;
        }
    }

    function username_exists($username){

        if ($stmt = $this->mysqli->prepare("SELECT username FROM users WHERE username=?")) {
            $stmt->bind_param("s", $username);
            $stmt->execute();
            $stmt->store_result();
            $count=$stmt->num_rows;
            $stmt->close();
        }
        return ($count > 0 ? true : false);
    }

    function signup($username, $password, $email, $capt_error){

    //validate username.
    if(!$this->valid_length($username)){
            $this->signup_error = "Username should be 5 - 15 characters.
";
    }else if(!$this->alpha_numeric($username)){
            $this -> signup_error ="Only letters, numbers";
    } else if($this->username_unallowed($username)){
            $this -> signup_error ="Not allowed.";
    } else if ($this->username_exists($username)){
            $this -> signup_error ="Username already exists.
";
    }

    //validate password.
   if (!$this->valid_length($password) ){
            $this -> signup_error .="Password should be 5 - 15 characters.
";
        }

    //validate email.
    if (!$this->valid_email($email) ){
            $this -> signup_error .="Invalid email.";
    }else if($this->email_exists($email)) {
            $this -> signup_error .="Email exists.
";
        }

    //captcha error
    if($capt_error == 1){
    $this -> signup_error .="Invalid captcha.
";
    }

    //no any error, proceed.
    if ($this->signup_error == ""){
        $hashed_password = $this->hash_password($password);
        if ($this->insert_new_user($username, $enc_password, $email)){
        $_SESSION['username']=$username;
        header("Location: welcome.php");
        }else {
        $this->signup_error = "Unknown error.
";
        }
    }

  }

}
?>

Why not use the built-in PHP e-mail validation?

filter_var($email, FILTER_VALIDATE_EMAIL)

Why do

if (lol())
  return true;
else
  return false;

when you can do

return lol();

Same goes for lol() ? true : false which you do too.

Furthermore the signup_error stuff is ugly. I'd suggest exceptions. At least make signup_error into an array instead, and just add new elements when you get an error. You can join() the array when outputting.

Also, you risk $count not being defined if $stmt fails.


A sleep primitive for CUDA device-side code I've so far been spared the need to waste any cycles on the GPU, but it seems like this might change. After getting some information I was missing about the semantics of CUDA's clock64() function, I've written the following bit of code for use in kernels (or other device-side functions):

// Shouldn't this be typedef'ed in the CUDA headers somewhere?
using clock_value_t = long long;

namespace detail {

__device__ void sleep(
    clock_value_t            num_cycles,
    volatile clock_value_t*  buffer_to_avoid_optimization)
{
    clock_value_t start = clock64();
    clock_value_t now;
    while (true) {
        now = clock64();
        // (Note the assumption of no wrap-around)
        clock_value_t cycles_elapsed = now - start;
        if (cycles_elapsed >= num_cycles) { break; }
    }
    // The memory write here should (hopefully) prevents the compiler
    // from optimizing the entire loop away
    *buffer_to_avoid_optimization = now - start;
}

} // namespace detail

/**
 * Have the executing warp busy-sleep until at least a certain
 * number of SM clock cycles have passed.
 *
 * @note The exact number of cycles busy-slept will depend on how soon the
 * warp will be scheduled to execute again after the last time it
 * iterates the elapsed-cycles check.
 *
 * @param num_cycles The minimum number of cycles to busy-sleep
 */
__device__ void sleep(clock_value_t num_cycles)
{
    static volatile clock_value_t buffer;
    detail::sleep(num_cycles, &buffer);
}

Other than general observations which are welcome, I have a few questions:

Is this approach reasonable, or should I be doing something very different?
Is there a better way of avoiding the while(true) loop from being optimized away?
Do I even need the volatile modifier on buffer_to_avoid_optimization?
Could I use a reference instead of a pointer?
Should I give it a different name?

If, as Robert Crovella suggests, the clock64() calls don't get optimized away, then this should be enough:

using clock_value_t = long long;

__device__ void sleep(clock_value_t sleep_cycles)
{
    clock_value_t start = clock64();
    clock_value_t cycles_elapsed;
    do { cycles_elapsed = clock64() - start; }
    while (cycles_elapsed < sleep_cycles);
}


Returning distinct characters in combined Strings I am trying to return distinct characters from the combination of 2 Strings. The following is my solution which seems to work based on my tests.

Is there a more efficient way of going about this? I have converted the strings to arrays, to sets, to a StringBuilder and back to a String. I'm wondering if I have overcomplicated something which could have been resolved a lot simpler.

public String longest (String s1, String s2) {

    String[] s1Arr = s1.split("");
    String[] s2Arr = s2.split("");

    Set<String> arr1 = new HashSet<>(Arrays.asList(s1Arr));
    Set<String> arr2 = new HashSet<>(Arrays.asList(s2Arr));

    arr1.addAll(arr2);

    StringBuilder sb = new StringBuilder();
    Iterator<String> it = arr1.iterator();
    while (it.hasNext()){
        String element = it.next();
        sb.append(element);
    }

    return sb.toString();
}

What comes to performance, you can improve it significantly:

public static String getDistinctCharString(String s1, String s2) {
    Set<Character> set = new HashSet<>(s1.length() + s2.length());

    char[] chars1 = s1.toCharArray();
    char[] chars2 = s2.toCharArray();

    for (char c : chars1) {
        set.add(c);
    }

    for (char c : chars2) {
        set.add(c);
    }

    StringBuilder sb = new StringBuilder(set.size());

    for (char c : set) {
        sb.append(c);
    }

    return sb.toString();
}

The point is that you deal with characters rather than substrings of length 1.

My demonstration gives me the following output:

longest() in 2558 ms.
getDistinctCharString() in 342 ms.
Algorithms agree: true

Hope that helps.


Leet code Majority element Problem :- Given an array of size n, find the majority element. The majority element is the element that appears more than ⌊ n/2 ⌋ times.
You may assume that the array is non-empty and the majority element always exist in the array.
Example 1:
Input: [3,2,3]
Output: 3
Example 2:
Input: [2,2,1,1,1,2,2]
Output: 2
Here is my solution
Approach 1
public static int majorityElement(int[] nums) {
    if (nums.length == 0) {
        return 0;
    }

    Map<Integer, Integer> numCountMap = new HashMap<>();

    int majorityElement = 0;

    for (int i : nums) {
        if (numCountMap.getOrDefault(i, 0) >= nums.length / 2) {
            majorityElement = i;
            break;
        }

        int currentCount = numCountMap.getOrDefault(i, 0);
        numCountMap.put(i, ++currentCount);
    }

    return majorityElement;
}

And here is my approach 2
 public int majorityElement(int[] nums) {
        Arrays.sort(nums);
        return nums[nums.length / 2];
    }

I was thinkin the first approach is better as the complexity would be n in first case and nlogn in 2nd case.
However, the result shows 2nd approach is better.
Please help me understand why 2nd approach is better and any suggestion for code for first and second.

This is not a review, but an extended comment.
Theoretically you are right, a time complexity of the first approach is \$O(n)\$. However, there is also a space complexity, also \$O(n)\$, due to the Map, and the map is a very heavyweight data structure. Even though it promises a constant time access, it is only an amortized constant time, and the constant could be quite large. You must also take into account its cache unfriendlyness.
Meanwhile, both approaches are suboptimal. There is a way to solve the problem in a linear time in constant space.


Changing a video link when hovered I have three divs that change a video link when they are hovered. I have the code working in three different blocks but I am not sure how to combine them since it is very repetitive.

$('#videolink-sd').mouseenter(function(event) {
    //$(this).toggleClass('home-pager-hover');

    $('.top-clock').stop( true, true ).fadeOut();
    $('#'+videoID).stop( true, true ).hide();

    $('#'+videoID).stop( true, true ).get(0).pause();
    $('#'+mp4ID).stop( true, true ).attr('src', sdmp4);
    $('#'+oggID).stop( true, true ).attr('src', sdogg);
    $('#'+videoID).stop( true, true ).get(0).load();
    $('#'+videoID).stop( true, true ).get(0).play();

    $('.top-clock').stop( true, true ).removeClass('top-clock--visible');
    $('.top-clock-sd').stop( true, true ).addClass('top-clock--visible');

    $('#'+videoID).stop( true, true ).delay('500').fadeIn();
    $('.top-clock-sd').stop( true, true ).delay('500').fadeIn();
});

$('#videolink-sk').mouseenter(function(event) {
    //$(this).toggleClass('home-pager-hover');

    $('.top-clock').stop( true, true ).fadeOut();
    $('#'+videoID).stop( true, true ).hide();

    $('#'+videoID).stop( true, true ).get(0).pause();
    $('#'+mp4ID).stop( true, true ).attr('src', skmp4);
    $('#'+oggID).stop( true, true ).attr('src', skogg);
    $('#'+videoID).stop( true, true ).get(0).load();
    $('#'+videoID).stop( true, true ).get(0).play();

    $('.top-clock').stop( true, true ).removeClass('top-clock--visible');
    $('.top-clock-sk').stop( true, true ).addClass('top-clock--visible');

    $('#'+videoID).stop( true, true ).delay('500').fadeIn();
    $('.top-clock-sk').stop( true, true ).delay('500').fadeIn();
});

$('#videolink-rb').mouseenter(function(event) {
    //$(this).toggleClass('home-pager-hover');

    $('.top-clock').stop( true, true ).fadeOut();
    $('#'+videoID).stop( true, true ).hide();

    $('#'+videoID).stop( true, true ).get(0).pause();
    $('#'+mp4ID).stop( true, true ).attr('src', rbmp4);
    $('#'+oggID).stop( true, true ).attr('src', rbogg);
    $('#'+videoID).stop( true, true ).get(0).load();
    $('#'+videoID).stop( true, true ).get(0).play();

    $('.top-clock').stop( true, true ).removeClass('top-clock--visible');
    $('.top-clock-rb').stop( true, true ).addClass('top-clock--visible');

    $('#'+videoID).stop( true, true ).delay('500').fadeIn();
    $('.top-clock-rb').stop( true, true ).delay('500').fadeIn();
});

$('.home-pager li').mouseenter(function(event){
    $('.home-pager li').stop( true, true ).removeClass('home-pager-hover');
    $(this).stop( true, true ).addClass('home-pager-hover');
});

It's been a while since I've done Javascript, but I think that you want something that looks like this:

function set_video_on_mouseenter(suffix, video_ID, mp4_ID, ogg_ID, mp4_value, ogg_value) {
  $('#videolink-' + suffix).mouseenter(function(event) {
    //$(this).toggleClass('home-pager-hover');

    $('.top-clock').stop( true, true ).fadeOut();
    $('#'+video_ID).stop( true, true ).hide();

    $('#'+video_ID).stop( true, true ).get(0).pause();
    $('#'+mp4_ID).stop( true, true ).attr('src', mp4_value);
    $('#'+ogg_ID).stop( true, true ).attr('src', ogg_value);
    $('#'+video_ID).stop( true, true ).get(0).load();
    $('#'+video_ID).stop( true, true ).get(0).play();

    $('.top-clock').stop( true, true ).removeClass('top-clock--visible');
    $('.top-clock-' + suffix).stop( true, true ).addClass('top-clock--visible');

    $('#'+video_ID).stop( true, true ).delay('500').fadeIn();
    $('.top-clock-' + suffix).stop( true, true ).delay('500').fadeIn();
  });
}

set_video_on_mouseenter('sd', videoID, mp4ID, oggID, sdmp4, sdogg);
set_video_on_mouseenter('sk', videoID, mp4ID, oggID, skmp4, skogg);
set_video_on_mouseenter('rb', videoID, mp4ID, oggID, rbmp4, rbogg);

$('.home-pager li').mouseenter(function(event){
    $('.home-pager li').stop( true, true ).removeClass('home-pager-hover');
    $(this).stop( true, true ).addClass('home-pager-hover');
});

There isn't enough code here to actually run, so I didn't test.  If you post a link to a page that actually has the code working, I can try to do more.

I didn't try to make the code more elegant.  This is a very mechanical refactor.  I looked at what changed and extracted that as well as variable uses from the original code.  Then I just call my new function with the extracted parameters.  You could probably do this without extracting the three ID variables, but I think this is more reliable since I can't verify that the code works.


Optimizing a table - Twig, CSS I have a table with forms in two colors - the first row is one color, the second is the other. Can you please tell me if there is a way to optimize my code (such as with this)?

I have two CSS classes:

tr.content1{
    background-color: #EFF4FA;
}

tr.content2{
    background-color: #F7FAFD;
}

And the table looks this way:

        Previous Plan:
        {{ form_row(form.prev_plan) }}

        Previous Server:
        {{ form_row(form.prev_srv) }}

There is also a problem this way - I get some text in the second column in front of the form field.

Depending on your requirements (is javascript allowed, which browsers can be used), I'd go for a CSS only solution.

CSS:

table.zebra tr:nth-child(odd) {
    background-color: #EFF4FA;
}

table.zebra tr:nth-child(even) {
    background-color: #F7FAFD;
}

PHP:

    Previous Plan:
    {{ form_row(form.prev_plan) }}

    Previous Server:
    {{ form_row(form.prev_srv) }}

Here's the catch:

You can't use these wonderfull selectors in every browser (IE < 9, I'm looking at you!). Only in most of them. There's a great explanation of the selector at w3c and an overview over browsers supporting it at caniuse.

And if you still want to use CSS3 selectors and need early IE support, there's javascript providing it (e.g. selectivizr - I never used it, but it's easy to find alternatives). Just search for "css3 selectors in IE" in your favorite search engine.

I guess this still doesn't solve your text overlay issues, but for that, you'd best point to an example page.


Getting totals of each primary key by matching it with another worksheet with a many relationship I have already resolved several others using Application.Index with Application.WorksheetFunction.Match and reduced time to perform from about 7-8 seconds to milliseconds. But I feel there is still room for improvement.

Should I use an array with Index and Match?

I was also told to use Scripting.Dictionary, but I am looking for someone who can demonstrate how to do it the right away in this scenario. Because in my head I have to populate the dictionary with a loop before I can even use that, so wont it be similar in terms of speed?

'Production Quantity for Dashboard
For i = 2 To Total_rows_Prod
    For j = 2 To Total_rows_Dash
        If ThisWorkbook.Worksheets("Prod. Qty.").Cells(i, 5) = ThisWorkbook.Worksheets("Dashboard").Cells(j, 1) Then
           ThisWorkbook.Worksheets("Dashboard").Cells(j, 4) = ThisWorkbook.Worksheets("Dashboard").Cells(j, 4) + ThisWorkbook.Worksheets("Prod. Qty.").Cells(i, 31) / ThisWorkbook.Worksheets("Prod. Qty.").Cells(i, 4)
        End If
    Next j
Next i

After doing some bottleneck testing as shown below (run time of code is shown at row 10):

However, when using Index and Match while only using 1 for-next loop as shown in the code below:

'Production Quantity for Dashboard
For i = 2 To Total_rows_Prod
    m = Application.Match(ThisWorkbook.Worksheets("Prod. Qty.").Cells(i, 5), ThisWorkbook.Worksheets("Dashboard").Range("A:A"), 0)
    If Not IsError(m) Then
        ThisWorkbook.Worksheets("Dashboard").Cells(Application.WorksheetFunction.Match(ThisWorkbook.Worksheets("Prod. Qty.").Cells(i, 5), ThisWorkbook.Worksheets("Dashboard").Range("A:A"), 0), 4) = ThisWorkbook.Worksheets("Dashboard").Cells(Application.WorksheetFunction.Match(ThisWorkbook.Worksheets("Prod. Qty.").Cells(i, 5), ThisWorkbook.Worksheets("Dashboard").Range("A:A"), 0), 4) + ThisWorkbook.Worksheets("Prod. Qty.").Cells(i, 31) / ThisWorkbook.Worksheets("Prod. Qty.").Cells(i, 4)
    End If
Next i

The run time would be negligible as shown below (still at row 10):

Any improvements that would make the time to perform optimally minimized would be great.

The final time I was able to make everything run with Index and Match replacements was 2 seconds:

But on a slower netbook running a Pentium Atom processor, it takes 26 seconds to perform the same code. So I am wondering if there is way to bring down that 26 seconds.

You've not provided a fully functional set of code with data, so I can only make untested suggestions as an example for you to incorporate into your application.

Without seeing the data that you are working with, it's impossible to know if a Dictionary is the best approach. Using a Dictionary is a good idea, but only if all of the data in the table is unique. In your code fragment, for example, you are relying on the comparison of two specific cells and are not searching for a value within the entire range of data (which is better suited to a Dictionary).

Using the Index or Match is also reasonable, but remember that these functions directly interact with the worksheet cells. With larger data sets in the Range, this results in slower execution because the Range object must be traversed to access the desired Cells.

Memory-based arrays would be my recommendation for the execution speed improvements you're looking for, with a straight up array-to-array comparison. You can learn more information about using arrays here and here.

Again, you didn't include a complete set of your code for review here, but there are a few observations I can make for improvement.

Always use Option Explicit. (You may be doing this, but we can't see it)
Create intermediate objects/variables to make your code more readable. As an example, you are repeating the full reference for a worksheet as ThisWorkbook.Worksheets("Prod. Qty.") everywhere.

While it might seem more tedious at first, trust me when I say it will make your code far more readable and easier to maintain if you set it up like this:

Dim prodQtyWS As Worksheet
Dim dashboardWS As Worksheet
Set prodQtyWS = ThisWorkbook.Worksheets("Prod. Qty.")
Set dashboardWS = ThisWorkbook.Worksheets("Dashboard")

Establish your ranges explicitly so the extent of your data is clear.

Again, it's a few extra lines of code, but it will not slow down the execution time. (My example here may be an incorrect reading of your OP, you'll have to make adjustments.)

Dim prodQtyRange As Range
Set prodQtyRange = prodQtyWS.Range("A2").Resize(Total_rows_Prod, 31)

From here on out it's implementing your business logic to update the dashboard data. Here is my example Sub, which again, you have to adapt to your specific application. You will see a significant speed improvement with this method.

Option Explicit

Sub Example(ByVal Total_rows_Prod As Long, ByVal Total_rows_Dash As Long)
    Dim prodQtyWS As Worksheet
    Dim dashboardWS As Worksheet
    Set prodQtyWS = ThisWorkbook.Worksheets("Prod. Qty.")
    Set dashboardWS = ThisWorkbook.Worksheets("Dashboard")

    '--- establish the ranges and copy the data into a memory array
    Dim prodQtyRange As Range
    Dim prodQtyData As Variant
    Set prodQtyRange = prodQtyWS.Range("A2").Resize(Total_rows_Prod, 31)
    prodQtyData = prodQtyRange.Value

    Dim dashboardRange As Range
    Dim dashboardData As Variant
    Set dashboardRange = dashboardWS.Range("A2").Resize(Total_rows_Dash, 31)
    dashboardData = dashboardRange.Value

    Dim i As Long
    Dim j As Long
    For i = 2 To Total_rows_Prod
        '--- calc this factor once, since we're adding the same value
        '    to all the dashboard rows
        Dim prodQtyFactor As Double
        prodQtyFactor = prodQtyData(i, 31) / prodQtyData(i, 4)
        For j = 2 To Total_rows_Dash
            If prodQtyData(i, 5) = dashboardData(i, 1) Then
                dashboardData(j, 4) = dashboardData(j, 4) + prodQtyFactor
            End If
        Next j
    Next i

    '--- copy the updated data back to the dashboard
    dashboardRange.Value = dashboardData

End Sub


Looking for a better solution than a bunch of switch statements in my JavaScript I'm calling this function from my html (and in some cases from another function in my JavaScript).  These type of switch statements are starting to litter my code.  Is there a more elegant solution to these if statements?  I wish I could pass a reference to the variables I want to modify.  I'm finally asking the question because now I have a situation where if I do it this same way, I'm going to have an ugly switch statement with hundreds of cases which I'm guessing not only looks ugly, but will hurt performance.

function incCriteriaStats(exp){
    switch (exp) {
      case "a":
        numOfA++;
        break;
      case "b":
        numOfB++;
        break;
      case "c":
        numOfC++;
        break;
      case "d":
        numOfD++;
        break;
      case "e":
        numOfE++;
        break;
      case "f":
        numOfF++;
        break;
      case "g":
        numOfG++;
        break;
    }
|

Use an object, with the keys being the letters, and then you can just do:

function incCriteriaStats(exp) {
        if(exp in obj)
             obj[exp]++;
}


Making adding to a dictionary more efficient? I have the following code and it's taking quite some time to run. I'm getting an Out of Memory Exception due to the high volume of answers there are.  Is there any way I could speed it up?

var controlStrings = answerControlStrings.Where(a => a.ControlN.Substring(a.ControlN.IndexOf('_') + 1, a.ControlN.Length - a.ControlN.IndexOf('_') - 1).Equals(newAnswer.QVar, StringComparison.InvariantCultureIgnoreCase));
if (controlStrings.Count() > 0)
{
    newAnswer.CatiControlStrings.Add("en", controlStrings.Where(c => c.CultureN.Contains("cati") && c.CultureN.Contains("en")).ToList());
    newAnswer.WebControlStrings.Add("en", controlStrings.Where(c => c.CultureN.Contains("web") && c.CultureN.Contains("en")).ToList());
    newAnswer.CatiControlStrings.Add("es", controlStrings.Where(c => c.CultureN.Contains("cati") && c.CultureN.Contains("es")).ToList());
    newAnswer.WebControlStrings.Add("es", controlStrings.Where(c => c.CultureN.Contains("web") && c.CultureN.Contains("es")).ToList());
}

You're iterating through controlStrings 4 times (because you have 4 Where clauses).

It might be better to rewrite this as a for loop:

foreach (var c in controlStrings)
{
    var cultureN = c.CultureN;

    if (cultureN.Contains("cati") && cultureN.Contains("en"))
        ... add c to CatiControlStrings;

    if (cultureN.Contains("web") && cultureN.Contains("en"))
        ... add c to WebControlStrings;

    ... and the same for Spanish.
}

You can also call Contains fewer times:

foreach (var c in controlStrings)
{
    var cultureN = c.CultureN;

    bool cati = cultureN.Contains("cati");
    bool web = cultureN.Contains("web");
    if (!cati && !web)
        continue;
    bool en = cultureN.Contains("en");
    bool es = cultureN.Contains("es");

    if (cati && en)
        ... add c to CatiControlStrings;

    if (web && en)
        ... add c to WebControlStrings;

    ... and the same for Spanish.
}

Also your Count() statement may be expensive and unnecessary.

You're calculating IndexOf twice for every string.

And you're creating a new Substring for every string you test. Instead of creating a substring, use one of the overloads of String.Compare which lets you compare substrings.

Alternatively something like this might be better (because it doesn't use Substring and is simpler than using IndexOf with Compare:

string compareWith = "_" + newAnswer.QVar;
var controlStrings = answerControlStrings.Where(a => a.ControlN.EndsWith(compareWith,
    StringComparison.InvariantCultureIgnoreCase));

Garbage, garbage, everywhere says that using a Where will generate a lot of temporary objects. I don't understand why you're getting "out of memory" sometimes, but rewriting even your first statement to use less Linq might help (or at least make the cause your "out of memory" exception clearer).

foreach (var a in controlStrings)
{
    if (a.ControlN.EndsWith(compareWith, StringComparison.InvariantCultureIgnoreCase))
    {
        // the add-to-dictionary code above
    }
}

Reading C# Does Lambda => generate garbage? (including comments to the accepted answer), Linq generates temporary garbage.

I have a suspicion, after reading Garbage Collection and Performance, that maybe the framework doesn't garbage-collect before throwing 'out of memory'; my guess is that:

Garbage collection runs when it feels like it
Garbage collection runs on a separate thread, without pausing/blocking your threads
Unlike Java, C# will/may not stop to garbage collect immediately before/instead of throwing 'out of memory exception'
Therefore if your thread generates too much garbage too fast (i.e. faster than the garbage is collected), it may throw.

An alternative theory is that garbage remains uncollectable inside the lambda instances until your var controlStrings goes out of scope.

Simplifying your code to use less Linq and no Substring instances should produce less garbage, which might help to avoid throwing the exception.


Validating Excel file columns I'm looking for ways to improve this code (more readable, less redundant and maybe cleaner/faster).

The problem I needed to solve:

I was designated to implement a software that is going to validate an 96 column Excel file and if there is no error on it, create an XML file from it. In case there are any error in the excel file I have to display them to the user and indicate where the problem occurred.

How I attempted to solve the problem:

Since I knew the numbers of columns, I thought about making a class that represents an cell on the Excel file and include a string property to hold an possibly error description, by making that it'd make it easy to display the error log. So I actually created 2 collections of my class that is called Cell.  One collection to hold all of Excel's cell values, and the other is the error log one.

Here is the Cell class code:

public class Cell
{
    public string Value { get; set; }
    public int Row { get; set; }
    public int Column { get; set; }
    public string ErrorDescription { get; set; }
}

This is the Cells collection:

Range worksheetCells = sheet.get_Range(firstCell, lastCell);
private List<Cell> Cells = new List<Cell>();

foreach (Range item in worksheetCells)
{
     Cells.Add(new Cell{ Value = item.Text, Row = item.Row, Column = item.Column});
}

For readability purposes, I create another 96 collections of Cell, one for each column:

var Name = Cells.Where(c => c.Column == 1);
....

I've created 96 methods of validation one per column/collection:

private void NameValidation(IEnumerable<Cell> excelColumn, List<Cell> log)
{
    foreach (Cell item in excelColumn)
    {
        if (string.IsNullOrEmpty(item.Value))
        {
            item.ErrorDescription = "You need to fill up this Cell.";
        }
        else
        {
            if (item.Value.Length > 27)
            {
                item.ErrorDescription = "The MAX length of this field is 27 characters.";
            }
        }

        if (!string.IsNullOrEmpty(item.ErrorDescription))
        {
            log.Add(item);
        }
    }
}

Basically that is what I have now.  I haven't touch the XML file creation yet, but the reason why I've designed my code like this is that I can use the validation method to create the XML, instead of having to do another 96 foreach statements.  I'd only create the XML file when after calling all the validation methods if the log.count return 0, so it is ok to generate the file.

What troubles my mind:

Do I really need to do 96 things all the time?

in 60% of the the validation methods I have to check for string.IsNullOrEmpty(item.Value) and copy and paste kills me, It makes me feel like I'm doing something wrong, you know?

In a few of those methods, the item.value is only mandatory if another cell is filled up.  If it is, then the current cell is mandatory. That's why I didn't make a single method to validate when the field is mandatory, because it depends on other things.  In 30% of the validation methods, I have to check for item.Value.Length > "something".

Do you have any ideas on how to take some of those if/else out?

Consider using custom attributes.
Here is a possible variant:
Create a class representing the data from the excel file and decorate it with attributes.

    public class ExcelData
    {
        [ExcelColumn("Name")]
        [Required]
        [MaxLength(27)]
        public string Name { get; set; }

        [ExcelColumn("NextColumn")]
        [Required]
        public string NextColumn { get; set; }
        //...another 94 property.
    }

Then create a class that gets excel and returns a list of this data using ExcelColumn attribute.

    public class ExcelParser
    {
        public IEnumerable<ExcelData> Parse(string fileName)
        {
            //Get file and return ExcelData, using ExcelColumn attribute.
        }
    }

Then create a class to convert this data to XML.

public class ExcelDataToXmlConverter
    {
        private ExcelDataValidator _validator = new ExcelDataValidator();

        public XDocument Convert(IEnumerable<ExcelData> excelData)
        {
            foreach (var data in excelData)
            {
                var errors = new Dictionary<string, List<string>>();
                if (_validator.IsValid(data, out errors))
                {
                    //Convert the row to XML
                }
                else
                {
                    //Do something with errors.
                }
            }
        }
    }

    public class ExcelDataValidator
    {
        //errors - is a list of errors where the key is the name of the field and value is the list of errors.
        public bool IsValid(ExcelData excelData, out Dictionary<string, List<string>> errors)
        {
            //Validate using different attributes: Required, MaxLength and so on.
        }
    }

Maybe you'll be able to use attributes for actual converting data to XML.
Of course it's only a very crude sketch, and only some parts of it may be useful for you, but I hope you've got the idea.


JSON to CSS in JavaScript I'm working on a JSON to CSS converter in NodeJS, which takes a .json file and generates a .css file with its utility classes from it.

.json example

    }
        "breakpoints-map": {
            "default": "0",
            "sm": "only screen and (min-width: 508px)",
            "md": "only screen and (min-width: 768px)"
        },
        "bg-repeat-map": {
            "bg-repeat": "repeat",
            "bg-repeat-y": "repeat-y",
            "bg-no-repeat": "no-repeat",
            "bg-repeat-x": "repeat-x"
        },
        "bg-position-map": {
            "bg-pos-top": "top",
            "bg-pos-right-top": "top right",
            "bg-pos-right-center": "right",
            "bg-pos-right-bottom": "right bottom",
            "bg-pos-bottom": "bottom",
            "bg-pos-left-bottom": "left bottom",
            "bg-pos-left": "left",
            "bg-pos-left-top": "left top",
            "bg-pos-center": "center center"
        },
        "bg-attachment-map": {
            "bg-att-scroll": "scroll",
            "bg-att-fixed": "fixed"
        },
        "border-style-map": {
            "border-dotted": "dotted",
            "border-dashed": "dashed",
            "border-solid": "solid",
            "border-double": "double"
        }
    }

.js code

    module.exports = (fileName) => {
        let cssWriter = fs.createWriteStream('style.css', {
            flags: 'a'
        });

        fs.readFile(fileName, (error, data) => {
            if (error) {
                functions.logIt("The file cannot be found or is unreadable.", error);
            } else {
                try {
                    const dataJson = JSON.parse(data);
                    const breakpointMap = dataJson["breakpoints-map"]
                    delete dataJson["breakpoints-map"];
                    Object.entries(breakpointMap).forEach(([breakpointKey, breakpointValue]) => {
                        if (functions.isDefault(breakpointKey) == false) {
                            cssWriter.write("@media " + breakpointValue + " {\n");
                        }
                        Object.entries(dataJson).forEach(([mapKey, mapValues]) => {
                            let breakpoint = (functions.isDefault(breakpointKey) == true ? "" : breakpointKey + "\\:");
                            let property = functions.getProperty(mapKey);
                            Object.entries(mapValues).forEach(([classKey, classValue]) => {
                                cssWriter.write("." + breakpoint + classKey + "{ " + property + ": " + classValue + "}\n");
                            })
                        })
                        if (functions.isDefault(breakpointKey) == false) {
                            cssWriter.write("}\n");
                        }
                    })
                } catch (error) {
                    functions.logIt("The file could not be parsed to JSON.", error);
                }
            }
            cssWriter.end();
        });
    };

The isDefault function just checks whether the given parameter is equal to "default", in order to not put a media query around it.

The getProperty function just links the right CSS property depending on the map name (width-map => width, bg-repeat-map => background-position).

The main problem I see is that it's not obvious what the output is because there's too much auxiliary code that effectively obfuscates the logic.

Offload some of the code into a function.
Use template strings.

If the data amount isn't in multi-megabyte range I would write a single string to improve readability:

const dataJson = JSON.parse(data);
const breakpointMap = dataJson['breakpoints-map'];
delete dataJson['breakpoints-map'];

const entryToCss = ([mapKey, mapValues], breakpoint) => {
  const property = functions.getProperty(mapKey);
  return Object.entries(mapValues)
    .map(([k, v]) => `.${breakpoint}${k}{ ${property}: ${v}}\n`)
    .join('');
};

cssWriter.write(
  Object.entries(breakpointMap).map(([bpKey, bpVal]) => {
    const breakpoint = functions.isDefault(bpKey) ? `${bpKey}\\:` : '';
    return `${
      breakpoint ? `@media ${bpVal} {\n` : ''
    }${
      Object.entries(dataJson)
        .map(entry => entryToCss(entry, breakpoint))
        .join('')
    }${
      breakpoint ? '}\n' : ''
    }`;
  }).join('')
);


Improving the manipulation of 2D Arrays for an easy challenge on HackerRank with Python 3 I am practicing some exercises on HackerRank in order to improve my skills on Python3. I am doing the 30 days challenge. There is one challenge involving matrices. I was able to pass the test cases and provide the correct result. However, I think my code could have huge improvements.

I am not really sure about how would be the best way to iterate over matrices to solve the problem asked. That's the challenge:

That's my code:

import sys

matrix_input = sys.stdin.readlines()
#print (matrix_input)

def make_matrix(matrix):

    final_matrix = []

    for line in matrix:

        #remove new_line
        line = line.strip()

        #remove space
        line = line.split(" ")

        #convert string to int
        line = list(map(int,line))

        final_matrix.append(line)

    return (final_matrix)

"""the function is going to receive a hour glass and return the sum of its elements. The easiest way is to treat things as a rectangle and make the elements on the second line, first col and last col, null, like zero"""

def sum_hour_glass_rec(hour_glass_rec):

    hour_glass_rec[1][0]=0

    hour_glass_rec[1][2]=0

    total = 0

    for linha in hour_glass_rec:
        for coluna in linha:
            total = total + coluna

    return total

def make_list_hour_glasses(matrix):

    A = matrix

    col_var= 0
    list_part_hour_glass_iter = []
    list_hour_glasses = []
    k_var_iter = 0

    for k in range(0,4):
        col_var = 0
        for j in range(0,4):
            #print (j,"j")
            for col in range(k_var_iter,k_var_iter+3):
                #print (col,"col")
                #print (A[col][col_var:col_var+3])
                list_part_hour_glass_iter.append(A[col][col_var:col_var+3])
                #print (list_part_hour_glass_iter)
                list_hour_glasses.append(list_part_hour_glass_iter)
            list_part_hour_glass_iter = []
            #print (list_hour_glasses)
            col_var+=1
        k_var_iter +=1

    return list_hour_glasses

matrix_of_recs = (make_list_hour_glasses(make_matrix(matrix_input)))

records = []

for rectangle in matrix_of_recs:
    records.append(sum_hour_glass_rec(rectangle))

maximum_record = max(records)

print(maximum_record)

In order to execute this, I created another file, called d11_input.txt with the input values:

1 2 3 4 5 6
7 8 9 10 11 12
13 14 15 16 17 18
19 20 21 22 23 24
25 26 27 28 29 30
31 32 33 34 35 36

My command on terminal would just be:

python3 d11.py < d11_input.txt

Points that I suspect could have huge improvements:

1 - I treated the hourglass as a square and then, made two elements A21 and A23 zero. Maybe there is a more elegant way to do that.

2 - I used a three nested loop in order to generate all hour glasses a 6x6 matrix can have. Not good in terms of big O notation. Is there a way better?

3 - As you might see, when generating all possible hour glasses, each one was repeated three times which means I am calculating things that I do not need. Nonetheless, I was not able to fix this and also keep the rest of the code working. Here you see a picture of the "problem" involving repeated data that I am talking about:

I hope I will learn with your comments. Thanks in advance =)

Generating all possible hourglasses

Don't :-) It's a lot of memory and processing churn.

A more efficient alternative is to write a function that takes as parameters a matrix and the starting position of an hourglass, and return the sum of values at the appropriate relative positions:

def compute_hourglass_sum(matrix, top, left):
    return sum(matrix[top][left:left+3]) + ...

The caller is responsible for passing parameters to valid hourglasses.

As for the unnecessary recomputing of sums of overlapping hourglasses, given the small sizes of hourglass dimensions, I don't think it's worth it to come up with something more clever.

However, if the dimensions became significantly bigger, then that would indeed make sense. Perhaps prefix sums would be useful for that. I suggest to check out the concept, it's very interesting stuff.

Processing the input

In make_matrix it's not so great that the input parameter is named matrix when in fact it's just unprocessed text. I'd name it input. And then the local final_matrix variable could be renamed to simply matrix.

And I would rewrite the function with a list comprehension:

return [map(int, line.split()) for line in input]

(Thanks @Graipher for simplifying line.strip().split(" ") to line.split().)

Minor things

There is an official coding style guide for Python called PEP8. I suggest to check it out. There is a command line tool that can check your code.

There are some redundant parentheses here and there. I suggest to remove them. The pep8 tool can pinpoint them for you.

The default start value of range(...) is 0, so instead of range(0, 4) you can write simply range(4).


A different way to execute a hangman game Most hangman implementations I have seen keep a list of characters the player has already guessed and compares them to the players current guess each time to see if they have already made the same guess.  My implementation checks if the word was changed after replacing any matches.  Is my implementation easier to understand as well as more efficient than this one Hangman game logicistics and cleanliness?

Side Question: Should I use static variables in loops?

Hangman

#include <iostream>
#include <fstream>
#include <cassert>
#include <vector>
#include <random>
#include <string>

std::string replace_if_match(std::string& write, const std::string& read, const std::string& match)
{
    assert( write.length() >= read.length() );

    for( int search_pos{0}; (search_pos = read.find( match, search_pos )) != -1; ++search_pos){

        write.replace( search_pos, match.length(), match );
    }
    return write;
}

int main()
{
    std::ifstream file("word_list");
    assert( file.is_open() );

    std::vector<std::string> word_list;

    while( !file.eof() ){

        static std::string word;
        file >> word;
        word_list.push_back( word );
    }
    file.close();

    std::random_device rd;
    std::mt19937 mt(rd());
    std::uniform_int_distribution<int> dist(0, word_list.size() - 1);

    std::string unknown_word;

    std::string previous_word;

    std::string guessed_word;

    std::string guess;

    int num_of_guesses;

    int in_game{true};

    while( in_game )
    {
        unknown_word = word_list[dist(mt)];

        guessed_word = std::string().append( unknown_word.length(), '_' );

        num_of_guesses = 5;

        while( num_of_guesses > 0 && unknown_word != guessed_word )
        {
            std::cout << guessed_word << '\n';

            std::cout << "Number of Guesses Left: " << num_of_guesses << '\n';

            std::cout << "Enter a string as guess.\n";
            std::cin >> guess;

            previous_word = guessed_word;

            if( previous_word == replace_if_match( guessed_word, unknown_word, guess ) ){

                num_of_guesses--;
            }
        }
        if( guessed_word == unknown_word ){

            std::cout << "We did not have a noose for you any way.\n";
        }
        else{

            std::cout << "You loser, the word was " << unknown_word << "!\n";
        }
        std::cout << "Would you like to try another? <Yes: 1, No: 0>\n";
        std::cin >> in_game;
    }
}

I find this function signature quite confusing:

std::string replace_if_match(std::string& write,
                             const std::string& read, const std::string& match)

The names write, read, and match aren't very descriptive and everything is being passed in by reference, so what is it going to be returning?

Changing the parameter names to something like guessed_word, word_to_guess and guess might be more descriptive.  The return still feels wrong though, why not just have it return true/false to indicate if replaces have been made?

The implementation you've made doesn't really conform to the standard rules of Hangman.  One of the advantages of keeping track of characters guessed is that it ensures you only guess one character at a time.  With your current approach, it's possible to guess partial words (so you can guess 'hang' and it would find the start of 'hangman').  Character matching also encourages you to think about casing, 'H' != 'h' with your current approach.  These choices may be intentional, but it feels unexpected.


Getting price for one firm by week I've a lot of data whit prices of a product from different firms by week. I've a class named WeekPrice witch contains a list of firms. Here you've my class structure.

Here you've a little part of the data in JSON format:

[
    {
        "ID": 1,
        "NumberOfWeek": 1,
        "Year": 2016,
        "Date": "01/04/2016",
        "Firms": [
            {
                "ID": 1,
                "Name": "Firm ABC",
                "Price": 0.73
            },
            {
                "ID": 2,
                "Name": "DEF Solutions",
                "Price": 0.97
            }
        ]
    },
    {
        "ID": 2,
        "NumberOfWeek": 2,
        "Year": 2016,
        "Date": "08/04/2016",
        "Firms": [
            {
                "ID": 3,
                "Name": "Firm ABC",
                "Price": 0.83
            },
            {
                "ID": 4,
                "Name": "DEF Solutions",
                "Price": 0.94
            }
        ]
    }
]

I'll to make a graph that shows the evolution of the price for one firm by week. to this I need to get the price and the week in a object. In the C# code below DoublePoint is used for do this. The property Data is the week number and Value is the price.

List<DoublePoint> list = (from week in MyListWitData
                          where week.Year == DateTime.Now.Year
                          group week by week.NumberOfWeek into grp
                          select new DoublePoint()
                          {
                              Data = grp.Key,
                              Value = (double)(from price in grp.ToList()
                                               where price.Year == DateTime.Now.Year
                                               select (from firm in prijs.Bedrijf
                                                       where firm.Name == "Firm ABC"
                                                       select firm.Price
                                                      ).FirstOrDefault<decimal>()
                                              ).FirstOrDefault()
                          }).ToList<DoublePoint>();

It works perfectly but the only problem is that the code is a little bit messy. Can you refactor my code so it's better in performance and easier to read it?

Thanks in advance

The model you chose for your data is quite far from ideal. I guess that's one of the reasons your code became a little messy. However, even with this model, the end result can be reached in a more readable, and probably more efficient way:

var pricesPerWeek = weekPrices
    .Where(weekPrice => weekPrice.Year == DateTime.Now.Year)
    .SelectMany(
        (weekPrice) => weekPrice.Firms.Where(firm => firm.Name == "ABC"),
        (week, firm) => new {week.Week, firm.Price})
    .ToList();

In this code, weekPrices is a List<WeekPrice>. This code first selects week prices from the current year. It then uses the overload SelectMany to flatten the list of week prices to a collection of anonymous objects with properties Week and Price.

A complete and working sample of this code can be found on dotnetfiddle.net.

However, as said before, your model is not ideal. I would refactor your model to three classes: Firm, Product and ProductPrice:

// Represents a firm you track.
public class Firm
{
    public Firm()
    {
        Prices = new List<ProductPrice>();
    }

    public int Id {get;set;}
    public String Name {get;set;}
    public List<ProductPrice> Prices {get; private set;}
}

// Keeps record of the firm's price of a product at a certain moment
public class ProductPrice
{
    public int FirmId {get;set;}
    public int ProductId {get;set;}
    public int Year {get;set;}
    public int Week {get;set;}
    public decimal Price {get;set;}
}

// Represents a product you track.
public class Product
{
    public int Id {get;set;}
    public String Description {get;set;}
}

This model has several advantages over your model. For instance, firms do not get a new Id every week. Instead, the firm's Id and Name are stored once. The same goes for products (although that isn't part of your question). All you have to do every week, is add an entry of type ProductPrice for all product-firm combinations.

And now, with this model, back to your question. To select the weeks and prices for a certain firm (and product) you can do:

var pricesPerWeek = firms
    .Where(firm => firm.Id == 1)
    .SelectMany(firm => firm.Prices)
    .Where(
        productPrice => productPrice.Year == DateTime.Now.Year &&
        productPrice.ProductId == 1)
    .Select(productPrice => new {productPrice.Week, productPrice.Price})
    .ToList();

In this code, firms is a List<Firm>. This code first selects the correct firm, it then selects the correct weeks (and product) and then returns a collection of anonymous objects with properties Week and Price.

A complete and working sample of this code can be found on dotnetfiddle.net.


Custom function to scroll spy on the jScrollPane plugin I have an element where the jScrollPane is being used to provide custom scrollbars for an extensive text. The idea behind the code is to change the image beside the text each time the user reaches a certain amount of scroll while reading the text.

To have the code flexible, I've prepared a method to detect the number of images available and from there establish the trigger points for the image change. This is all based on the text wrapper height.

Can't this be simplified, thus reducing the amount of code, perhaps in such reduction, have it improved?

HTML structure

jQuery/JavaScript code

$imagesWrap = $('#imageWrap');

if ($imagesWrap.size()==1) {

    //target all images and count them
    var $targets = $imagesWrap.find('> img'),
        total    = $targets.size();

    //for each found, set the CSS z-index
    $targets.each(function(){

        var $this = $(this),
            index = $this.data('index');

        //revert order
        $this.css({
            'z-index': total-index
        });
    });

    var $scrollSpy   = $('.jspPane'),           //the element to spy
        height       = $scrollSpy.height(),     //its height
        stepsTrigger = parseInt(height/total),  //trigger at "every" this distance
        currentPos   = 0;                       //current position (active image)

    setInterval(function(){

        //ascertain current position (insure positive number)
        currentPos = Math.abs(parseInt($scrollSpy.css('top'), 10));

        //ascertain the target image
        var img  = Math.round(currentPos/stepsTrigger),
            id   = img==0 ? 1 : img,
            $tgt = $('img[data-index="'+id+'"]');

        //if different from the already active
        if (!$tgt.hasClass('active')) {

            $imagesWrap.find('.active').removeClass('active').hide();
            $tgt.fadeIn().addClass('active');
        }

    }, 100);
}

I'd like to address a couple of style concerns over simplification.

Purely for debugging if you have function expressions then name them.

eg.
    setInterval(function(){

could become

setInterval(function _scrollDetectionInterval(){

When debugging the call stack will now show you at a glance what this function is (especially helpful if you have multiple).
you have define variables outside of the scope you are using them.

var $scrollSpy   = $('.jspPane'),           //the element to spy
height       = $scrollSpy.height(),     //its height
stepsTrigger = parseInt(height/total),  //trigger at "every" this distance
currentPos   = 0;                       //current position (active image)

This doesn't make sense unless you aren't going to change them again or you will be using them else where. Assuming this is all the code here I would at least put the currentPos inside the interval function scope.
you have a data-index="1" attribute on your elements. Is this actually needed? jQuery has an index you can use in the .each() method. just change your function to

$targets.each(function(index){

unless I am missing something it should work. You can also then change the line

$tgt = $('img[data-index="'+id+'"]');

to

$tgt = $targets.eq(Math.max(0, img - 1));

there is a lot going on in this line. $targets.eq( uses the 0-based index of the element in the array. Math.max( ensures it will never be less than 0.

The code will only work if there is one image wrapper on the page. If you are looking to extend this to multiple here is how i would change it:

$('.someImageWrapClass')
  .each(function _eachWrapper()
  {
    var $parentWrapper = $(this);

    //target all images and count them
    var $targets = $parentWrapper.find('> img'),
        total    = $targets.size();

    //for each found, set the CSS z-index
    $targets.each(function _eachImage(index){
        //revert order
        $(this).css({
            'z-index': total - index
        });
    });

    var $scrollSpy   = $('.jspPane'),           //the element to spy
        height       = $scrollSpy.height(),     //its height
        stepsTrigger = parseInt(height/total);  //trigger at "every" this distance

    setInterval(function _scrollDetection()
    {
        //current position (active image)
        //ascertain current position (ensure positive number)
        var currentPos = Math.abs(parseInt($scrollSpy.css('top'), 10));

        //ascertain the target image
        var img  = Math.round(currentPos/stepsTrigger)
            $tgt = $targets.eq(Math.max(0, img -1));

        //if different from the already active
        if (!$tgt.hasClass('active'))
        {
            $targets.removeClass('active').hide();
            $tgt.fadeIn().addClass('active');
        }

    }, 100);
  });


C# Async and DataContext access I'm using a DataContext to insert an object. Currently it's working, but I'm feeling things could be done better. Method is called from UI (Using Caliburn Micro Binding). IsBusy is a property used by the UI (Xceed Extended WPF Toolkit) to display a wait message.

Repository inherits from DataContext (using Entity Code First).

public async void AddTemplate()
{
    IsBusy = true;

    await Task.Run(() => {
        using (var repo = new Repository())
        {
            var template = new Template() { ... };
            try
            {
                repo.Templates.Add(template);
                repo.SaveChanges();
                this.TryClose(true);
            }
            catch (SqlException ex)
            {
                _util.Log.AddException(Exceptions.DB_EXC, ex.Message);
            }
            catch (EntityException ex)
            {
                _util.Log.AddException(Exceptions.EF_EXC, ex.Message);
            }
            catch (Exception ex)
            {
                _util.Log.AddException(Exceptions.US_EXC, ex.Message);
            }
            finally
            {
                IsBusy = false;
            }
        }
        this.TryClose(false);
    });
}

I'm feeling odd about covering using with the await Task.Run since I can also await for repo.SaveChangesAsync(), but then IsBusy will not change (and the Wait message will not be made visible).

You should be able to await the repo.SaveChangesAsync() method.

public async Task AddTemplateAsync()
{
    var template = new Template {...};

    using (var repo = new Repository())
    {
        try
        {
            IsBusy = true;
            repo.Templates.Add(template);
            await repo.SaveChangesAsync();
        }
        catch {...}
        finally
        {
            IsBusy = false;
            TryClose();
        }
    }
}

Since AddTemplate() is an async method, it should be called AddTemplateAsync() and return a Task. Take a look at these best practices for more details. You might want to consider passing in a CancellationToken depending on your requirements.

I would also advise against swallowing the base Exception type for reasons explained here.