Displaying orders and customers in a table, filtering out duplicates Can I make any further improvements to simplify / speed up this code? My reason for creating new arrays for the variables $name, $business, $email, $orderstatus is because when displaying results there will be identical order ID numbers stored in $orderid.

My script filters out the duplicates then with the remaining results matches up the keys from $orderid with the correct key in $name, $business, $email, $orderstatus

 <?php
session_start();
if (!isset($_SESSION['username'])){
header("location:login.php");
}
// DB Credentials
$dbusername = 'xxxx';
$dbpassword = 'xxxx';

// Connect to order database
$conn = new PDO('mysql:host=localhost;dbname=xxxx_orders', $dbusername, $dbpassword);
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

// Select Order IDs from database
try {
$stmt = $conn->prepare('SELECT  o.*, c.fname, c.lname, c.business FROM xxxx_orders.orders o, xxxx_customers.customers c WHERE c.email = o.email');
$stmt->execute();
}
catch(PDOException $e) {
    echo "I'm sorry, xxxx. I'm afraid I can't do that.";
    file_put_contents('PDOErrors.txt', $e->getMessage(), FILE_APPEND);
}

// For each result create or add to the arrays $orderid,$name,$business,$email,$orderstatus
foreach ($stmt as $result) {

$orderid[]=$result['orderid'];
$name[]=$result['fname'] . " " . $result['lname'];
$business[]=$result['business'];
$email[]=$result['email'];
$orderstatus[]=$result['orderstatus'];

}

// Filter out duplicate order IDs from $orderid array
$orderid = array_unique($orderid);

// Display results section

foreach ($orderid as $key => $value) {
        echo "
";

        // Display First/Last name from array $name with $key being equal to the corresponding key from array $orderid.
        // The key is necessary to properly display the correct result
        echo "" . $name[$key] . "	";

        // Display business name from array $name with $key being equal to the corresponding key from array $orderid.
        // The key is necessary to properly display the correct result
        echo "" . $business[$key] . "	";

        // Display email from array $name with $key being equal to the corresponding key from array $orderid.
        // The key is necessary to properly display the correct result
        echo "" . $email[$key] . "	";

        // Display order status from array $name with $key being equal to the corresponding key from array $orderid.
        // The key is necessary to properly display the correct result
        echo "" . $orderstatus[$key] . "	";

        // Display First/Last name from array $name with $key being equal to the corresponding key from array $orderid.
        // The key is necessary to properly display the correct result
        echo "" . $orderid[$key] . "	";

        echo "
";
}
?>

There are several major problems.

You've mixed presentation with logic.
You have some error/exception handling but not enough.
Where is your PDOErrors.txt being written to? It looks like it's being written to your web directory. This allows someone to visit http://yoursite.com/PDOErrors.txt and view information that may allow them to compromise your site.
You read from variables without ensuring they've been initialized. Even though php does not require variables to be initialized, I've found that this is a great place for bugs to live.
The way you are iterating over query results and creating arrays makes no sense. I know you've mentioned there are duplicates to remove but your solution is not the way to handle that situation. As others have mentioned, you either have problems with your schema that should be corrected, or you should use sql to return the proper result set.
You should not display unlimited result sets. The app might seem like it works now but, when your data grows the app will eventually crash due to either time or memory exhaustion. Result sets should be paginated with sane limits.
Your code is vulnerable to cross site scripting (or at least broken html). Whenever you do echo $foo, you should instead do echo htmlspecialchars($foo).

I know you've stated you are new to programming so I understand and sympathize with wanting to write code this way. You get quick results and from your perspective and I'm sure it seems like it works. But, as your app grows, you will find that this style is completely un-maintainable. Even you, the author will not be able to follow or understand it (not to mention the poor sap who comes after you).

Additionally, you have introduced some major security vulnerabilities without even realizing it. My advice is that you should immediately abandon this script and look to one of the many excellent frameworks for php (symfony or zend to name a few). These kinds of frameworks already contain what you need to write secure and organized applications. Although it would take time to learn how to use a framework, it is far less time than you would spend maintaining scripts written this way and dealing with the security problems.


Filtering array of objects I built a search bar that will filter an array of objects. But I feel like I can improve it. When I filter, I want to filter the first and last name case insensitive, then sort the names and then map them to be rendered.

I'm using typescript so ignore SelectedProps:

return contacts
    .filter((item: SelectedProps) => (item.firstName + item.lastName).toLowerCase().includes(search.toLowerCase()))
    .sort((a: SelectedProps, b: SelectedProps) => a.firstName > b.firstName ? 1 : -1)
    .map((item: SelectedProps, index: number) => (
     // IGNORE OUTCOME
    ));

If someone searches for "Firstname Lastname" (with a space) it won't find it, even if it exists. I would suggest removing spaces from the search string with search.replace(/ /g, '')

You are not doing anything for special characters like apostrophe. Should searching for "OConner" not return "O'Connor"?

The sort function you have is almost the same as the default sort, except yours doesn't return 0 if they are equal. But you shouldn't use that anyway, since they are case sensitive. You can use .localeCompare instead.

Also note the IE doesn't support .includes. If this is running in a browser and you're not already using a polyfill or transpiler, you should do that, or find another way if you want to support IE.


Basic Color class for both Delphi and Lazarus Intro
If you want to know, then about 10+ years ago, I started a journey on the (best in my country) color picker for WinXP, later Win7. Since now it is hardly compatible with Win10 and HiDPI (work in progress), and in no way compilable in Linux using Lazarus, it would take quite an effort to create a decent desktop color picker with zoom and everything on Linux. But first things first - in this question I would like to start with basic color class, i.e. class capable of holding single color, and providing easy access to each color component, except for hue sat lum, which I'm not able to program, and am not asking you to point me to any direction. At the bottom I attached this code to existing color picker for windows just to show what it is doing. I want to start from scratch and maybe re-invent the wheel just to practice my brain.

TBasicColor class
unit basic_color;

interface

uses
    Graphics;

type
    TBasicColor = class
    strict private
        FColorRef: TColor;
    protected
        function GetColorRef: Integer;
        function GetRed: Byte;
        function GetGreen: Byte;
        function GetBlue: Byte;
        function GetCyan: Byte;
        function GetMagenta: Byte;
        function GetYellow: Byte;
    public
        constructor Create; reintroduce;
        constructor CreateRandom;
        constructor CreateColorUsingRGB(const ARed, AGreen, ABlue: Byte);
        constructor CreateColorUsingCMY(const ACyan, AMagenta, AYellow: Byte);
        property ColorRef: Integer read GetColorRef;
        property Red: Byte read GetRed;
        property Green: Byte read GetGreen;
        property Blue: Byte read GetBlue;
        property Cyan: Byte read GetCyan;
        property Magenta: Byte read GetMagenta;
        property Yellow: Byte read GetYellow;
    end;

implementation

constructor TBasicColor.Create;
begin
    inherited Create;
    // implicitly initialize to white color
    CreateColorUsingRGB(255, 255, 255);
end;

constructor TBasicColor.CreateRandom;
begin
    inherited Create;
    FColorRef := Random($FFFFFF + 1);
end;

constructor TBasicColor.CreateColorUsingRGB(const ARed, AGreen, ABlue: Byte);
begin
    inherited Create;
    FColorRef := ARed or (AGreen shl 8) or (ABlue shl 16);
end;

constructor TBasicColor.CreateColorUsingCMY(const ACyan, AMagenta, AYellow: Byte);
begin
    CreateColorUsingRGB(255 - ACyan, 255 - AMagenta, 255 - AYellow);
end;

function TBasicColor.GetColorRef: Integer;
begin
    Result := Integer(FColorRef);
end;

function TBasicColor.GetRed: Byte;
begin
    Result := Byte(FColorRef);
end;

function TBasicColor.GetGreen: Byte;
begin
    Result := Byte(FColorRef shr 8);
end;

function TBasicColor.GetBlue: Byte;
begin
    Result := Byte(FColorRef shr 16);
end;

function TBasicColor.GetCyan: Byte;
begin
    Result := 255 - GetRed;
end;

function TBasicColor.GetMagenta: Byte;
begin
    Result := 255 - GetGreen;
end;

function TBasicColor.GetYellow: Byte;
begin
    Result := 255 - GetBlue;
end;

end.

Screenshot from Windows

Whatever you answer, I value all input. Thank you.

Self-review

Randomize missing
Since I use Random to generate pseudo-random color, it is, I lightly remember, a must to call Randomize once program or unit in this case is being created; more information.
// ...

initialization
    Randomize;

end.

Get... Red, Green, Blue, Cyan, Magenta, Yellow
These seem to operate as expected.

constructors
The biggest error I already see I have made is the constructor thing. The problem is I did not write one for direct TColor assignment. It could be re-written for instance like this in the interface:
type
    TBasicColor = class
    // ...
    public
        // default direct TColor assignment constructor
        constructor Create(const AColor: TColor); overload;
        // reintroduce is hiding TObject constructor
        constructor Create; reintroduce; overload;
        // create using RGB values
        constructor CreateRGB(const ARed, AGreen, ABlue: Byte);
        // create using CMY values
        constructor CreateCMY(const ACyan, AMagenta, AYellow: Byte);
        // create pseudo-random color constructor
        constructor CreateRandom;
    // ...

Plus, like this in the implementation:
constructor TBasicColor.Create(const AColor: TColor);
begin
    // in here it is just plain assignment
    inherited Create;
    FColorRef := AColor;
end;

constructor TBasicColor.Create;
begin
    // in case anyone just calls Create() we assign white color
    Create($FFFFFF);
end;

constructor TBasicColor.CreateRGB(const ARed, AGreen, ABlue: Byte);
begin
    Create(ARed or (AGreen shl 8) or (ABlue shl 16));
end;

constructor TBasicColor.CreateCMY(const ACyan, AMagenta, AYellow: Byte);
begin
    CreateRGB(255 - ACyan, 255 - AMagenta, 255 - AYellow);
end;

constructor TBasicColor.CreateRandom;
begin
    Create(Random($FFFFFF + 1));
end;

As you can see, all, in the end, are calling the default constructor, which I see as much better implementation.

overload keyword
Note on overload keyword, I originally did not need it in Lazarus, but Delphi requires it.

Comments
By the way, I should really use more comments, they will prove useful one day, once I read it after years.

Why read-only ColorRef?
On second thought, I see no reason for the ColorRef not being able to change at runtime, I find it hard to see what reason I had before, but no matter, it should stay a private member with properties to safely read and write, also the typecast might be wrong, cannot confirm or disprove at this point, best to typecast when necessary in-place.
For example with private method Assign:
procedure TBasicColor.Assign(const ColorRef: TColor);
begin
    if (ColorRef < 0) or (ColorRef > $FFFFFF)
        then raise ERangeError.Create('ERangeError in TBasicColor class.' + sLineBreak +
                                      'It supports only subset of TColor range.' + sLineBreak +
                                      'Valid range is <0; $FFFFFF>.')
        else FColorRef := ColorRef;
end;

which can in turn be used in the SetColorRef setter:
procedure TBasicColor.SetColorRef(const ColorRef: TColor);
begin
    Assign(ColorRef);
end;

ARed change to Red, etc.
I believe it's a habit or style point, but anyway.
I removed, and am no longer a fan of an A prefixing, changed to this:
constructor TBasicColor.CreateRGB(const Red, Green, Blue: Byte);
constructor TBasicColor.CreateCMY(const Cyan, Magenta, Yellow: Byte);

Modified code
After a few other adjustments, I will name only the use of setters in all color components, this unit could be re-written finally to this state:
unit basic_color;

interface

uses
    Graphics, SysUtils;

type
    TBasicColor = class
    strict private
        FColorRef: TColor;
    private
        // TColor assignment with range check <0; $FFFFFF>
        procedure Assign(const ColorRef: TColor);
        // independent function needed (Delphi/Lazarus; Windows/Linux)
        function RGBToColor(const Red, Green, Blue: Byte): TColor;
    protected
        function GetColorRef: TColor;
        procedure SetColorRef(const ColorRef: TColor);
        function GetRed: Byte;
        procedure SetRed(const NewRed: Byte);
        function GetGreen: Byte;
        procedure SetGreen(const NewGreen: Byte);
        function GetBlue: Byte;
        procedure SetBlue(const NewBlue: Byte);
        function GetCyan: Byte;
        procedure SetCyan(const NewCyan: Byte);
        function GetMagenta: Byte;
        procedure SetMagenta(const NewMagenta: Byte);
        function GetYellow: Byte;
        procedure SetYellow(const NewYellow: Byte);
    public
        // default direct TColor assignment
        constructor Create(const ColorRef: TColor); overload;
        // reintroduce is hiding TObject default constructor
        constructor Create; reintroduce; overload;
        // create color using RGB values
        constructor CreateRGB(const Red, Green, Blue: Byte);
        // create color using CMY values
        constructor CreateCMY(const Cyan, Magenta, Yellow: Byte);
        // create pseudo-random color
        constructor CreateRandom;
        property ColorRef: TColor read GetColorRef write SetColorRef;
        property Red: Byte read GetRed write SetRed;
        property Green: Byte read GetGreen write SetGreen;
        property Blue: Byte read GetBlue write SetBlue;
        property Cyan: Byte read GetCyan write SetCyan;
        property Magenta: Byte read GetMagenta write SetMagenta;
        property Yellow: Byte read GetYellow write SetYellow;
    end;

implementation

procedure TBasicColor.Assign(const ColorRef: TColor);
begin
    if (ColorRef < 0) or (ColorRef > $FFFFFF)
        then raise ERangeError.Create('ERangeError in TBasicColor class.' + sLineBreak +
                                      'It supports only subset of TColor range.' + sLineBreak +
                                      'Valid TBasicColor range is <0; $FFFFFF>.')
        else FColorRef := ColorRef;
end;

function TBasicColor.RGBToColor(const Red, Green, Blue: Byte): TColor;
begin
    Result := Red or (Green shl 8) or (Blue shl 16);
end;

constructor TBasicColor.Create(const ColorRef: TColor);
begin
    // in here it is just plain assignment
    inherited Create;
    Assign(ColorRef);
end;

constructor TBasicColor.Create;
begin
    // in case anyone just calls Create() we assign white color
    Create($FFFFFF);
end;

constructor TBasicColor.CreateRGB(const Red, Green, Blue: Byte);
begin
    Create(RGBToColor(Red, Green, Blue));
end;

constructor TBasicColor.CreateCMY(const Cyan, Magenta, Yellow: Byte);
begin
    CreateRGB(255 - Cyan, 255 - Magenta, 255 - Yellow);
end;

constructor TBasicColor.CreateRandom;
begin
    Create(Random($FFFFFF + 1));
end;

function TBasicColor.GetColorRef: TColor;
begin
    Result := FColorRef;
end;

procedure TBasicColor.SetColorRef(const ColorRef: TColor);
begin
    Assign(ColorRef);
end;

function TBasicColor.GetRed: Byte;
begin
    Result := Byte(FColorRef);
end;

procedure TBasicColor.SetRed(const NewRed: Byte);
begin
    Assign(RGBToColor(NewRed, GetGreen, GetBlue));
end;

function TBasicColor.GetGreen: Byte;
begin
    Result := Byte(FColorRef shr 8);
end;

procedure TBasicColor.SetGreen(const NewGreen: Byte);
begin
    Assign(RGBToColor(GetRed, NewGreen, GetBlue));
end;

function TBasicColor.GetBlue: Byte;
begin
    Result := Byte(FColorRef shr 16);
end;

procedure TBasicColor.SetBlue(const NewBlue: Byte);
begin
    Assign(RGBToColor(GetRed, GetGreen, NewBlue));
end;

function TBasicColor.GetCyan: Byte;
begin
    Result := 255 - GetRed;
end;

procedure TBasicColor.SetCyan(const NewCyan: Byte);
begin
    SetRed(255 - NewCyan);
end;

function TBasicColor.GetMagenta: Byte;
begin
    Result := 255 - GetGreen;
end;

procedure TBasicColor.SetMagenta(const NewMagenta: Byte);
begin
    SetGreen(255 - NewMagenta);
end;

function TBasicColor.GetYellow: Byte;
begin
    Result := 255 - GetBlue;
end;

procedure TBasicColor.SetYellow(const NewYellow: Byte);
begin
    SetBlue(255 - NewYellow);
end;

initialization
    Randomize;

end.


Optimal method for writing bytes to file I'm making a method to save a byte[] to a file. It's for a Java Helper I'm writing, so it needs to be able to handle any kind of system. I saw on this example the method of FileOutputStream called write which accepts a byte[]. But I already have a method which will save an input stream to a file.

Which is best?

Alternative 1

/**
* Saves the given bytes to the output file.
*
* @param bytes
* @param outputFile
* @throws FileNotFoundException
* @throws IOException
*/
public static void saveBytesToFile(byte[] bytes, File outputFile) throws FileNotFoundException, IOException {
  ByteArrayInputStream inputStream = new ByteArrayInputStream(bytes);
  saveInputStream(inputStream, outputFile);
}

/**
* Saves the given InputStream to a file at the destination. Does not check whether the destination exists.
*
* @param inputStream
* @param destination
* @throws FileNotFoundException
* @throws IOException
*/
public static void saveInputStream(InputStream inputStream, File outputFile) throws FileNotFoundException, IOException {
  try (OutputStream out = new FileOutputStream(outputFile)) {
    byte[] buffer = new byte[2097152];
    int length;
    while ((length = inputStream.read(buffer)) > 0) {
      out.write(buffer, 0, length);
    }
    inputStream.close();
  }
}

Alternative 2

/**
* Saves the given bytes to the output file.
*
* @param bytes
* @param outputFile
* @throws FileNotFoundException
* @throws IOException
*/
public static void saveBytesToFile2(byte[] bytes, File outputFile) throws FileNotFoundException, IOException {
  FileOutputStream out = new FileOutputStream(outputFile);
  out.write(bytes);
}

Obviously the second is shorter and easier, but I'm just wondering whether one's more optimal than the other. Also, as a side note, I'm wondering about the optimal size for the byte buffer.

Given the care and effort that has been put into performance within the JVM and Java standard libraries, it is a virtual certainty that the second implementation will be faster.

However, does it really matter? Worrying about optimization is generally only useful if the system performance is inadequate. It's far more important to worry about functionality and readability first. Performance issues can be tackled (if they exist) when the system is nearing completion and it is possible to get actual performance data on where the bottlenecks are.

See the Wikipedia article on Program Optimization - especially the "When to optimize" section


Iterative uniform recombination of integer lists in python with numba I have a list of N integers and I iteratively take two integers at random, which are not at the same position in the list and uniformly recombine their binary representation, e.g. in intList = [1,4,1,5] (N=4) the number on the second and third position is chosen, aka 4 & 1, and their binary represenations 4=100 & 1=001 is uniformly recombined. Uniformly recombined means that I chose either the 1 or the 0 of the first position, one of the two 0's of the second position and either the 0 or 1 of the third position. This could result in 000 or 101 or 100 or 001. The result is saved as integer in the list. In each iteration I do this recombination for all N integers. This happens in a function with a numba decorator. My code is:
@nb.njit()
def comb():
    iterations = 100000
    N = 1000
    intList = list(range(N))   # this is just an example of possible integer lists
    l = 10   # length of the largest binary representation.
    intList_temp = [0]*N
    for _ in range(iterations):
        for x3 in range(N):
            intList_temp[x3] = intList[x3]
        for x3 in range(N):
            randint1 = random.randint(0, N - 1)
            randint2 = random.randint(0, N - 1)
            while randint1 == randint2:
                randint1 = random.randint(0, N - 1)
                randint2 = random.randint(0, N - 1)
            a = intList[randint1]
            b = intList[randint2]
            c = a ^ ((a ^ b) & random.randint(0, (1 << l) - 1))
            intList_temp[x3] = c
        for x3 in range(N):
            intList[x3] = intList_temp[x3]
    return intList

print(timeit(lambda: comb(), number=1))
>>>2.59s

My question is, can this be improved?

No significant performance improvement but cleaner code.
Because temp_list is overwritten element-wise you can create it once and then leave it. At the end of each iteration you can then copy the entire list into int_list for the next iteration.
Similarly you can simplify creating the random ints a and b a bit. There are a lot of ways that get close to this in numpy but nothing I can find that beats the naive sampling directly works with numba and numba beats any overall solution I can think of without it.
Unfortunately (or maybe fortunately depending on your view), the numba compiler is compiling it down to the best solution I can think of for both your original version and this. Only difference is readabilitiy:
@nb.njit()
def comb(int_list, l, iterations):
    n = len(int_list)

    temp_list = int_list

    for _ in range(iterations):
        for i in range(n):
            a, b = 0, 0
            while a == b:
                a = random.randint(0, n - 1)
                b = random.randint(0, n - 1)

            temp_list[i] = a ^ ((a ^ b) & random.randint(0, l))

        int_list = temp_list

    return int_list

print(timeit(lambda: comb(list(range(1000)), (1 << 10) - 1, 100000), number=10))


Producer/Consumer implementation using condition_variable Trying to implement producer/consumer problem in C++. I have one question: should I use nofify_all or notify_one method here? Also any feedback is welcome

template <class T>
class Stack
{
public:
    Stack(size_t size) : size_(size)
    {
        arr_ = new T[size_];
        init();
    }

    ~Stack()
    {
        delete[] arr_;
    }

    void init()
    {
        top_ = -1;
    }

    int capacity()
    {
        return static_cast<int>(size_) - top_ - 1;
    }

    void set(T num)
    {
        std::unique_lock<std::mutex> locker(mu);

        cond.wait(locker, [this](){ return top_ < static_cast<int>(size_) - 1; });
        top_++;
        arr_[top_] = num;
        std::cout << "Setting num: " << num << " Capacity: " << this->capacity() << std::endl;
        locker.unlock();
        cond.notify_one();
    }

    Т get()
    {
        std::unique_lock<std::mutex> locker(mu);
        T top_value;

        cond.wait(locker, [this](){ return top_ != -1; });
        top_value = arr_[top_];
        top_--;
        std::cout << "Getting num: " << top_value << " Capacity: " << this->capacity() << std::endl;
        locker.unlock();
        cond.notify_one();

        return top_value;
    }

private:
    size_t size_;
    T* arr_;
    int top_;
    std::mutex mu;
    std::condition_variable cond;
};

main:

int main()
{
    Stack<int> stack(10);
    std::vector<std::thread> producers;
    std::vector<std::thread> consumers;

    for (int i = 0; i <= 20; i++)
    {
        consumers.push_back(std::thread([&](){
            stack.get();
        }));
    }

    for (int i = 1; i <= 20; i++)
    {
        producers.push_back(std::thread([&](){
            stack.set(i);
        }));
    }

    for (auto& th : producers)
    {
        th.join();
    }

    for (auto& th : consumers)
    {
        th.join();
    }

    return 0;
}

A couple of comments on your test-harness code:

for (int i = 0; i <= 20; i++)
// ...
for (int i = 1; i <= 20; i++)

You've got 21 consumers and only 20 producers. This means your test code will never terminate, right?

You can avoid off-by-one errors like this by getting comfortable with the "half-open range" idiom of C-style languages. Every for-loop you write in C, C++, Java, C#, JavaScript, Perl, or whatever, should look like this:

for (int i = 0; i < N; ++i)

Notice that it starts at 0 (inclusive) and goes up to N (exclusive), so it iterates exactly N times.

    consumers.push_back(std::thread([&](){
        stack.get();
    }));

is a verbose way of writing

    consumers.emplace_back([&](){ stack.get(); });

I see you writing

arr_ = new T[size_];

— that's a red flag. Prefer to use std::vector<T> or std::unique_ptr<T[]> instead of raw pointers. The reason is that those will behave properly on copy-construction and/or move-construction, whereas your current pointer-based code requires some extra code in the special member functions (constructors and assignment operators) in order to work correctly.

Look up the Rule of Zero and try to apply it here.

Since you're relying on the destructor to delete[] the array allocated in your constructor, you should =delete the copy constructor and copy assignment operator; otherwise you're allowing the client code to make shallow copies that will lead to use-after-free and double-free bugs.

But! Your current code does actually result in an implicitly deleted copy constructor, because the default definition would have been ill-formed, because you can't copy-construct a std::condition_variable. So you're okay, modulo some really verbose error messages and the danger that someone might come along and refactor your class to remove the uncopyable members.

Shouldn't a Stack be copyable? or at least movable? Right now your Stack is neither copyable nor movable, because of its std::condition_variable and std::mutex members.

The names set and get (for push and pop respectively) are ill-chosen.

The name capacity for space_left is also ill-chosen. The word capacity already has a meaning in C++: it means "maximum possible size", as in std::vector::capacity. When you're trying to find a name for a thing, choose the name that best represents the thing itself — usually this is easy because most things already have established names (e.g. stack, push, pop). When you need a name for a thing that really doesn't yet have a name, then you just need to avoid naming it the same as something else that's already in the lexicon (e.g. capacity).

capacity needs to be a private member function, or else it needs to take the mutex mu, because otherwise the client code could call x->capacity() at the same time as some other thread is calling x->get() and suddenly you've got a race condition.

However, if you simply slap a mutex lock around the body of capacity, you'll find that set and get both deadlock, because each of them calls capacity while the mutex mu is already (still) locked.

std::cout << "Getting num: " << top_value << " Capacity: " << this->capacity() << std::endl;
locker.unlock();

Calling such expensive I/O routines under a mutex is a bad habit anyway. You could at least improve the code slightly by rephrasing it as

int cap = this->capacity();
locker.unlock();
std::cout << "Getting num: " << top_value << " Remaining capacity: " << cap << std::endl;

Consider what happens when T is not default-constructible; consider what happens when T is default-constructible but its default constructor is relatively expensive. What do you want to have happen in those cases?

Consider what happens when T is move-only. What do you want to have happen in that case?

Consider what happens when the following line in set throws:

top_++;
arr_[top_] = num;  // T::operator= THROWS AN EXCEPTION

What happens the next time you get from the stack? What do you want to have happen in that case?

But basically, this seems like reasonably good code (modulo the wrong names for things like push and pop, and the for-loops).

Your use of mutexes and condition variables seems fine to me. notify_one will probably be more efficient than notify_all, but either one is fine.


Make planet  spin on click I'm attempting to get an image to spin a certain amount when a separate div is clicked - in this case, a 72 degree rotation.

I've added clickable divs in the top right and left of the viewport, that should make the planet image spin 72 degrees on each click.

However, it only spins once. Unless I click it before the end of the animation, which is a weird effect. Can you please review this code and let me know why I can only spin once?

https://codepen.io/Punchyoual/pen/oGRLpX

HTML:

CSS:

body {
  background-image:url('https://space-facts.com/wp-content/uploads/magellanic-clouds.png');
  overflow:hidden;
  display: flex;
  justify-content: center;
  align-items: center;
}

#left-button {
  position:fixed;
  top:0px;
  left:0px;
  height:50px;
  width:50px;
  background-color:red;
}

#right-button {
  position:fixed;
  top:0px;
  right:0px;
  height:50px;
  width:50px;
  background-color:red;
}

#inner-orbit {
  margin:0 auto;
  width:150vw;
  height:145vw;
  text-align:center;
  background-image:url('https://my-rewards.co.uk/s3/W1siZiIsIjIwMTcvMTAvMjMvMTUvNDMvNTAvODE3L3BsYW5ldHMyXzAxLnBuZyJdXQ?sha=069b6bb83dffffde');
    background-size: 100% 100%;
    background-repeat: no-repeat;
  /*-webkit-animation: spin-right 25s linear infinite;
  animation: spin-right 25s infinite;
  transform:rotate(72deg);*/
}

#inner-orbit{transition: all 2s ease-in-out;}

@-webkit-keyframes spin-right {
  100% {
    -webkit-transform: rotate(360deg);
    -moz-transform: rotate(360deg);
    -ms-transform: rotate(360deg);
    -o-transform: rotate(360deg);
    transform: rotate(360deg);
  }
}

JS:

var degrees = 0;

$("#right-button").click(function() {

  degrees += 72;

  //$('.img').addClass('rotated'); // for one time rotation

  $("#inner-orbit").css({
    transform: "rotate(" + degrees + "deg)",
    "-ms-transform": "rotate(" + degrees + "deg)",
    "-moz-transform": "rotate(" + degrees + "deg)",
    "-webkit-transform": "rotate(" + degrees + "deg)",
    "-o-transform": "rotate(" + degrees + "deg)"
  });

  console.log(degrees);
});

$("#left-button").click(function() {

  degrees -= 72;

  //$('.img').addClass('rotated'); // for one time rotation

  $("#inner-orbit").css({
    transform: "rotate(" + degrees + "deg)",
    "-ms-transform": "rotate(" + degrees + "deg)",
    "-moz-transform": "rotate(" + degrees + "deg)",
    "-webkit-transform": "rotate(" + degrees + "deg)",
    "-o-transform": "rotate(" + degrees + "deg)"
  });

  console.log(degrees);
});

I'm not a smart man.

Just figured out that I was covering the buttons with the #orbit div...


Calculating business logic Here's a messy C# method used to calculate some business logic.

Does anybody have any suggestions on how I can optimise this code? I'm asking in case there are bad practices, improvements or functions within .NET/LINQ which I'm missing.

private static decimal GetRecurringCostEquiv(List<QuotationItemViewModel> items)
{

    var discVal = GetDiscounts(items, (int)RecordEnums.ProductClassification.Service);

    decimal? currentRecurringCosts = GetRecurringCost(items).Value;

    decimal? result = 0.00M;

    decimal highestContractTerm = items.Where(x => x.ProductTypeId == (int)RecordEnums.ProductClassification.Service).Select(y => y.RecurringTerm).Max() ?? 0;

    if ((discVal > 0) && (highestContractTerm > 0))
    {
        var calc = items.Where(x => x.ProductTypeId == (int)RecordEnums.ProductClassification.Service).Select(y => y.RecurringCostCore * y.Quantity);
        decimal total = calc.Sum() ?? 0;
        decimal totalOverTerm = total * highestContractTerm;
        decimal totalOverTermMinusDiscounts = totalOverTerm - discVal;
        result = totalOverTermMinusDiscounts / highestContractTerm;
    }
    else
    {
        result = currentRecurringCosts;
    }

    return result ?? 0.00M;
}

There's two things that I'm seeing right off the bat.

Don't use "magic integers" where you can be using Enums.  GetDiscounts should probably receive an enum, not an int:

var discVal = GetDiscounts(items, RecordEnums.ProductClassification.Service);
You don't need to Select then Max.  Just using Max will give you the desired outcome.

decimal highestContractTerm = items.Where(x => x.ProductTypeId == (int)RecordEnums.ProductClassification.Service).Max(y => y.RecurringTerm ?? 0;


Match two dictionary keys efficiently in Python I have two dictionaries that I need the keys to match and if they match i will get the value of the first dictionary and then get the value of 2nd dictionary and build a new dictionary out of those.

dic1 = {u'ShutterHours': 732, u'ShieldHours': 732, u'MaskHours': 28, u'TargetHours': 790, u'GndRingHours': 25}
dic2 = {u'ShutterHours': u'Shutter Hours', u'ShieldHours': u'Shield Hours', u'MaskHours': u'Mask Hours', u'TargetHours': u'Target Hours', u'GndRingHours': u'Ground Ring Hours'}

This is my code so far:

final = {}
for k,k2 in zip(dic1,dic2):
    if k == k2:
        final[dic2[k]] = dic1[k]

Expected output:

{u'Ground Ring Hours': 25, u'Shutter Hours': 732, u'Shield Hours': 732, u'Mask Hours': 28, u'Target Hours': 790}

This works fine but I'm a bit skeptical if this is the best and Pythonic way to do this.

This mechanism is so fragile that I can't tell for sure what it is supposed to do.  The main issue is that you can't reliably predict the order in which the keys of a dictionary will be enumerated.  For example, this seems to work as you intend:

>>> dic1 = {u'ShutterHours': 732, u'ShieldHours': 732, u'MaskHours': 28, u'TargetHours': 790, u'GndRingHours': 25}
>>> dic2 = {u'ShutterHours': u'Shutter Hours', u'ShieldHours': u'Shield Hours', u'MaskHours': u'Mask Hours', u'TargetHours': u'Target Hours', u'GndRingHours': u'Ground Ring Hours'}
>>> list(zip(dic1, dic2))
[('ShutterHours', 'ShutterHours'), ('ShieldHours', 'ShieldHours'), ('GndRingHours', 'GndRingHours'), ('MaskHours', 'MaskHours'), ('TargetHours', 'TargetHours')]

However, if you insert one entry into dic2, the whole thing might fall apart:

>>> …
>>> dic2[''] = 'empty string'
>>> list(zip(dic1, dic2))
[('ShutterHours', 'ShutterHours'), ('ShieldHours', ''), ('GndRingHours', 'GndRingHours'), ('MaskHours', 'ShieldHours'), ('TargetHours', 'MaskHours')]

What you probably want is a dict comprehension.

final = {dic2[k]: v for k, v in dic1.items()}

Better variable names would make the code easier to understand as well (e.g. labels instead of dic2).


Generating random strings I've created the following string manipulation function for randomizing my passed string in Lua:

require "string"
require "math"

math.randomseed( os.time() )

function string.random( self )
    local tTemporary, tNew = {}, {}
    if not self or self:len() < 5 then
        return nil
    end
    self:gsub( "%a", function( cChar )
            table.insert( tTemporary, cChar )
        end
    )
    for i = 1, #tTemporary, 1 do
        local iRandom = math.random(1, #tTemporary)
        tNew[i] = tTemporary[iRandom]
        table.remove( tTemporary, iRandom )
    end
    return table.concat(tNew, "   ")
end

Can this be optimized/more-randomised?

A couple of points to consider:

Calling your function shuffle instead of random would be a better name since that's really what it's doing.
The extra intermediate tables aren't really necessary to perform the shuffle. A single table is enough and you can perform the shuffle in-place right on that table.
Consider a better separation between the process of shuffling and how the input is stored. For example, if you have your shuffle function take a table instead then that function can be reused on any kind of data and not just strings.
I would do away with the Hungarian notation and just drop the type prefix on the variable names. Remember Lua variables themselves doesn't have a type. If you later change what that variable refers to then the Hungarian notation becomes misleading.

So I would refactor your original code into two functions. Here's one possibility:

function shuffle(self)
  if type(self) ~= 'table' then return nil end

  for i = #self, 2, -1 do
    local randi = math.random(i)
    self[i], self[randi] = self[randi], self[i]
  end
  return self
end

function shuffle_words(str)
  local strtable = {}
  for each in str:gmatch("%a+") do
    table.insert(strtable, each)
  end
  return table.concat(shuffle(strtable), ' ')
end


Roman Numerals Kata in C Here is a "Roman Numerals" kata implementation. Given a decimal number one has to return a corresponding roman number.

I have following questions:

Is memory management correct? Here I assume that the caller is responsible for freeing memory.
Does this code conform to c code style?
Does it look like idiomatic c?

Thanks!

toRoman.h:

char * toRoman(int n);

typedef struct DecimalToRoman {
    int decimal;
    const char * roman;
} DecimalToRoman;

toRoman.c:

char * toRoman(int n) {
    if (n < 1) return NULL;

    const DecimalToRoman numbers[] = {
        { 1000, "M"  },
        {  900, "CM" },
        {  500, "D"  },
        {  400, "CD" },
        {  100, "C"  },
        {   90, "XC" },
        {   50, "L"  },
        {   40, "XL" },
        {   10, "X"  },
        {    9, "IX" },
        {    5, "V"  },
        {    4, "IV" },
        {    1, "I"  }
    };

    size_t resultLen = 1;
    int tmp = n;
    for (size_t i = 0; i < sizeof numbers / sizeof(DecimalToRoman) && tmp; i++) {
        while(tmp >= numbers[i].decimal) {
            tmp -= numbers[i].decimal;
            resultLen += strlen(numbers[i].roman);
        }
    }

    char * result = malloc(resultLen);
    if (result == NULL) {
        perror("allocate");
        return NULL;
    }
    memset(result, '\0', resultLen);

    for (size_t i = 0, r = 0; r < sizeof numbers / sizeof(DecimalToRoman); r++) {
        while (n >= numbers[r].decimal) {
            n -= numbers[r].decimal;

            int j = 0;
            while (numbers[r].roman[j])
                result[i++] = numbers[r].roman[j++];
        }
    }
    return result;
}

toRoman.h

Is there any reason to expose DecimalToRoman to callers?

char * toRoman(int n)

Nit: it might be preferable to take unsigned rather than int as you cannot really handle negative numbers.

if (n < 1) return NULL;

Seems reasonable.  If you took unsigned, then you would only have to handle 0.  In that case, it might make sense to return "" so you always return a valid string.

On second thought, there's sort of an upper-bound on what you can reasonably write as a roman numeral.  Does it make sense to write two billion in terms of thousands?  If you choose an upper bound, you can make assumptions about the length of your buffer.

const DecimalToRoman numbers[] = {
....

I love that this is written out so clearly.  Nice.

size_t resultLen = 1;

Help a reader out by saying why you init to 1.  IMO it's not obvious until you get to malloc.

int tmp = n;
for (size_t i = 0; i < sizeof numbers / sizeof(DecimalToRoman) && tmp; i++) {
    while(tmp >= numbers[i].decimal) {
        tmp -= numbers[i].decimal;
        resultLen += strlen(numbers[i].roman);
    }
}

How about making this a function?  There's a common pattern:

Type thing = init;
...actually figure out what thing is often with a loop...

// don't change thing anymore!
... use thing ...

This is almost always more clearly written as:

Type const thing = createThing(init);
... use thing ...

But anyway back to this code.

sizeof numbers / sizeof(DecimalToRoman)

How about making this a variable named lenNumbers or something like that?  Super nit: I like that you used parens for the type and not for the expression, but IMO it looks weird with the operator/.  More clear: (sizeof numbers) / sizeof(DecimalToRoman).

if (result == NULL) {
    perror("allocate");
    return NULL;
}

Good.  I admit I usually assert in situations like this since there's not much you can do when you're out of memory (and it rarely happens in the things I do), but this is the party line.

memset(result, '\0', resultLen);

Isn't this overkill?  You're about to overwrite the memory anyway.  Could just set the last character to '\0` (and you might as well do that after you write the answer for cache locality).

        int j = 0;
        while (numbers[r].roman[j])
            result[i++] = numbers[r].roman[j++];

Better option: a for loop.  Best option: strcpy.

You could remove the calls to strlen by storing the length in DecimalToRoman.  The lengths are tiny so this is more a question of simplicity than optimization.

There's a common pattern in C APIs for returning buffers. The idea is you let the user pass in the buffer and you return an error code.  Something like:

int getRomanNumerals(int nInput, char* strOut, int* nLen);

The return value is either success, failure, or "insufficient buffer" (you probably want an enum or a preprocessor #define for this).  In any case, the user passes in a buffer and the buffer's length.  You always set nLen to the number of bytes needed, and you set strOut if you have enough space (maybe you start setting strOut assuming the buffer is long enough, but stop writing if you run out of space and only need to set nLen).

This way the user can do crazy stuff like store the result in a weird place or maybe the user knows they don't have enough memory to store the whole thing etc.


Find the number of quadraples (a,b,c,d) from 4 respective arrays such that a+b+c+d=0 Given four lists of numbers, calculate how many ways there are to
  choose one element from each to get a sum of zero.

  The input is given on stdin, first the length of all lists n, then n
  tuples containing an element from each, like this:

6
-45 22 42 -16
-41 -27 56 30
-36 53 -37 77
-36 30 -75 -46
26 -38 -10 62
-32 -54 -6 45

The task on SPOJ is here.

 #include <cstdio>
 #include <algorithm>
 using namespace std;
 #define MAX 10000001

    int arx[MAX];
    int ary[MAX];

     int main(){
     int n;scanf("%d",&n);
     int tmp1[n],tmp2[n],tmp3[n],tmp4[n];
     int n2=n*n;

     for(int i=0;i<n;i++)
        scanf("%d%d%d%d",&tmp1[i],&tmp2[i],&tmp3[i],&tmp4[i]);

     int count=0;
     for(int i=0;i<n;i++)
     {
        for(int j=0;j<n;j++)
        {
        arx[count]=tmp1[i]+tmp2[j];
        ary[count]=tmp3[i]+tmp4[j];
        count++;
        }
     }

     sort(arx,arx+n2);
     sort(ary,ary+n2);

     long long sum=0;

     for(int i=0;i<n2;i++)
     {   int key=-arx[i];
         int low1=0,high1=n2-1;
         while(low1<=high1)
      {
             int mid=(low1+high1)/2;
             if(ary[mid]<key)low1=mid+1;
             else high1=mid-1;
      }

         if(low1<n2&&ary[low1]==key)
         {
           int low2=0,high2=n2-1;
           while(low2<=high2)
           {
             int mid=(low2+high2)/2;
             if(ary[mid]>key)high2=mid-1;
             else low2=mid+1;
           }

         if(high2>=0&&ary[high2]==key)
         sum+=high2-low1+1;
        }

     }
     printf("%lld",sum);
     printf("\n");
    return 0;
}

I made an array(arx[]) to store all possible a+b and an array(ary[]) to store all possible c+d; Then I used binary-search to find no of occurrences of each (-)element of arx[] in ary[]. I narrowly passed the time limit.

I suspect there's a better way to do it.

Well, you are going about it wrong.

As a wise man once said: If in doubt, sort.

Read all four lists.
Sort them.
Calculate the smallest and biggest value which can be compensated by the lists after the first m.
Iterate over those elements of the first list which can be compensated according to 3.

Iterate over those elements of the second list which can be compensated according to 3, keeping in mind your choice for 4.

Repeat step 5 for the third list, keeping in mind your choice for 5.

Add the number of elements in the last list which can compensate the previous choices to the result.

Aside from that:

Your indentation is very haphazard.
using namespace std; is a good way to shoot yourself in your own foot, and makes any toolchain-change unpredictable.
Dynamically-sized automatic arrays aren't C++, they are C99+. Try for std::vector or such.
return 0; is implicit for main in C++.

My own solution, which can still easily be optimized further. Don't peek before you have it.


MessageFormat.format() with named parameters I was looking for how to use MessageFormat.format() in Java with named parameters, as a similar option exists in Python, and found this question. Some functions are presented there, although they use internal cycles and StringBuilder, and apparently they can fall into infinite loops (comments there suggest that, and it makes sense).

Option 1 (potential infinite loop):

public static String format(String str, Map<String, Object> values) {

    StringBuilder builder = new StringBuilder(str);

    for (Entry<String, Object> entry : values.entrySet()) {

        int start;
        String pattern = "%(" + entry.getKey() + ")";
        String value = entry.getValue().toString();

        // Replace every occurence of %(key) with value
        while ((start = builder.indexOf(pattern)) != -1) {
            builder.replace(start, start + pattern.length(), value);
        }
    }

    return builder.toString();
}

Option 2 (infinite loop removed):

public static String dictFormat(String format, Hashtable<String, Object> values) {
    StringBuilder convFormat = new StringBuilder(format);
    Enumeration<String> keys = values.keys();
    ArrayList valueList = new ArrayList();
    int currentPos = 1;
    while (keys.hasMoreElements()) {
        String key = keys.nextElement(),
        formatKey = "%(" + key + ")",
        formatPos = "%" + Integer.toString(currentPos) + "$";
        int index = -1;
        while ((index = convFormat.indexOf(formatKey, index)) != -1) {
            convFormat.replace(index, index + formatKey.length(), formatPos);
            index += formatPos.length();
        }
        valueList.add(values.get(key));
        ++currentPos;
    }
    return String.format(convFormat.toString(), valueList.toArray());
}

And the question is, why not something simpler? Why is there an internal cycle?
Here my option (which probably has some issue I'm not aware of).

public static String format(String str, Map<String, String> values) {

    String finalString = str;

    // validation input string
    if (str == null || str.isEmpty()) {
        return finalString;

    // validate parameters
    } else if (values == null || values.isEmpty()) {
        return str;
    }

    for (Map.Entry<String, String> entry : values.entrySet()) {

        String pattern = "%(" + entry.getKey() + ")";
        String value = entry.getValue();

        // Replace every occurrence of %(key) with value
        finalString = finalString.replace(pattern, value);
    }

    return finalString;
}

Performing string substitutions using multiple passes is almost always the wrong approach, and leads to bugs.  If one of the values happens to be a string that looks like a %(key), then all sorts of unpredictable things could happen, including various uncontrolled format string attacks!

Therefore, the string replacements must be done in a single pass of the format string.  I recommend doing it using a regular expression.

Furthermore, your method provides no escape mechanism, in case you need to specify a literal %(blah) in the format string.  In Java, it would be customary to use backslash as an escape character.

Suggested solution

This solution uses Matcher.replaceAll(Function<MatchResult,String> replacer), which was introduced in Java 9, to provide each substitution text via a callback.

import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class NamedFormatter {
    private static final Pattern RE = Pattern.compile(
        "\\\\(.)" +         // Treat any character after a backslash literally
        "|" +
        "(%\\(([^)]+)\\))"  // Look for %(keys) to replace
    );

    private NamedFormatter() {}

    /**
     * Expands format strings containing %(keys).
     *
     *
Examples:

     *
     *

     *
NamedFormatter.format("Hello, %(name)!", Map.of("name", "200_success")) → "Hello, 200_success!"

     *
NamedFormatter.format("Hello, \%(name)!", Map.of("name", "200_success")) → "Hello, %(name)!"

     *
NamedFormatter.format("Hello, %(name)!", Map.of("foo", "bar")) → "Hello, %(name)!"

     *

     *
     * @param fmt The format string.  Any character in the format string that
     *            follows a backslash is treated literally.  Any
     *            %(key) is replaced by its corresponding value
     *            in the values map.  If the key does not exist
     *            in the values map, then it is left unsubstituted.
     *
     * @param values Key-value pairs to be used in the substitutions.
     *
     * @return The formatted string.
     */
    public static String format(String fmt, Map<String, Object> values) {
        return RE.matcher(fmt).replaceAll(match ->
            match.group(1) != null ?
                match.group(1) :
                values.getOrDefault(match.group(3), match.group(2)).toString()
        );
    }
}


Counting increasing subsequences with a "hacker's" Binary Index Tree This is an \$O(n \sqrt n)\$ solution to the the following problem:

  Given a sequence, compute the number of non-empty increasing subsequences

The algorithm is to compute g(i) = # of increasing subsequences that end at index i using dynamic programming, and then sum over i. A full description of the algorithm used can be found here. However, naively using dynamic programming yields an \$O(n^2)\$ solution, which is too slow. It's possible to get \$O(n \log n)\$ using a BIT, but I opted for the \$O(n \sqrt n)\$ "hacker's" BIT, implemented below. As I am a novice (relatively), my question is: how could I have implemented it better? Specifically, I generally prefer to program in a more functional style, but this code is very imperative. How could I have made it more functional?

from math import sqrt
from bisect import bisect_left

def count_sequences(seq):
    # Normalize seq
    seq_sorted = sorted(seq)
    seq = list(map(lambda x: bisect_left(seq_sorted, x), seq))

    # Initialize the memoization arrays
    n = len(seq); f = [0]*n
    nn = int(sqrt(len(seq))); ff = [0]*(nn+2)

    # Compute g(i) = f(s[i])
    for s in seq:
        res = sum([
            1,
            sum(ff[: (s // nn)]),
            sum(f[(s // nn)*nn : s])
        ])

        f[s] += res
        ff[s // nn] += res

    # Sum all g(i)
    return sum(ff)

I wouldn't go as far to say that functional programming and Python don't mix, but you should really consider what you're going for while writing it whatever language you are writing in. I would not make your goal:

  How could I have made it more functional?

while writing Python. If you are used to writing in Haskell (for example) you shouldn't expect to always write like that in Python, so I think you should reconsider your ideal.

As for an actual correction to your code:

If you're going to call list on map, you might as well use a list comprehension, change:

seq = list(map(lambda x: bisect_left(seq_sorted, x), seq))

to:

seq = [ bisect_left(seq_sorted, x) for x in seq ]


Temperature converter that seems to violate SRP I've written this class for two purposes (I think only this fact shows that this code violates SRP): convert weight from one unit to another and to represent them accurate or not accurate. So class methods can be used for calculation and the present method rounds the value for presentation. I don't like this code - can you propose better solution?

class Temperature

  UNITS = %w(C F)
  DEFAULT_UNITS = "C"

  attr_reader :value, :units

  def initialize(value, units = DEFAULT_UNITS)
    @value = value
    @units = units.to_s
  end

  def as(units, accurate)
    return unless UNITS.include?(units.to_s)
    value = units.to_s == @units ? @value : self.class.try("#{@units.downcase}_to_#{units.downcase}", @value)
    accurate ? value : self.class.new(value, units).present
  end

  def default(accurate = false)
    as(DEFAULT_UNITS, accurate)
  end

  def present
    value.round(1)
  end

  def self.c_to_f(val)
    val * 9 / 5 + 32
  end

  def self.f_to_c(val)
    (val - 32) * 5 / 9
  end

end

class Numeric

  def fahrenheit_to_celsius
    (self - 32) * 5 / 9
  end

  def celsius_to_fahrenheit
    self * 9 / 5 + 32
  end

end

p -7.5.celsius_to_fahrenheit #=> 18.5

Classes are open for a reason in ruby. A temperature is just a number, converting it is just a method. The accuracy thing disappears by itself - an integer results in an integer, a float will give a float.


Rust: exercise of employees and names As I'm learning Rust using the second edition of the Rust book I solved the following exercise:

  Using a hash map and vectors, create a text interface to allow a user to add employee names to a department in a company. For example, “Add Sally to Engineering” or “Add Amir to Sales.” Then let the user retrieve a list of all people in a department or all people in the company by department, sorted alphabetically.

I could find this solution: Rust exercise department employee management (which did not receive a lot of feedback).

Do you have any feedback on my implementation? The code works, but I can imagine that some refactoring would be good. It already took me a lot of trial and error to get this working :-)

Suggestions welcome!

use std::io;
use std::collections::HashMap;

fn main() {
    let mut employees:HashMap<String, Vec<String>> = HashMap::new();
    println!("Start adding employees or type 'Done' when finished!");
    loop {
        let mut input = String::new();
        io::stdin().read_line(&mut input).expect("error: unable to read user input");
        let parsed_input = dissect_input(input.clone());
        match parsed_input {
            Ok(v) => {
                let (dept, name) = v;
                employees.entry(dept.to_string())
                    .or_insert_with(Vec::new)
                    .push(name)
            }
            Err(e) => {
                if e == "End" {
                    println!("Processed all input, good!\n\n");
                    break;
                } else {
                    println!("Input error!");
                    continue;
                }
            }
        }
    }
    println!("Type the name of a department to get its employees.");
    println!("Type 'All' to get all employees by department.");
    println!("Type 'Quit' to quit.");
    loop {
        let mut command = String::new();
        io::stdin().read_line(&mut command).expect("error: unable to read user input");
        match command.trim() {
            "Quit" => break,
            "All" => {
                for (dept, names) in &employees {
                    let mut names = names.clone();
                    names.sort();
                    for name in names {
                        println!("{}: {}", dept, name);
                    }
                }
            }
            _ => {
                match employees.get(&command.trim().to_string()) {
                    Some(names) => {
                        for name in names { println!("{}: {}", command.trim().to_string(), name); }
                    }
                    None => println!("I don't recognize that!")
                    }
                }
            }
        }
    println!("Have a nice day!");
    }

fn dissect_input(s: String) -> Result<(String, String), String> {
    match s.trim().as_ref() {
        "Done" => Err("End".to_string()),
        _ => {
            let words: Vec<&str> = s.split_whitespace().collect();
            if words.len() != 4 {
                Err("PEBCAK".to_string())
            } else {
            Ok((words[3].to_string(), words[1].to_string())) // “Add Sally to Engineering”
            }
        }
    }

}

// Using a hash map and vectors, create a text interface to allow a user
// to add employee names to a department in a company.
// For example, “Add Sally to Engineering” or “Add Amir to Sales.”
// Then let the user retrieve a list of all people in a department
// or all people in the company by department, sorted alphabetically.
```

To start, I have some general tips. Run your code through rustfmt to get consistent formatting and make code easier to read. Run your code through clippy to get tips on common mistakes (clippy had nothing to say about your code, nice!)

One overall thing I'd suggest is to not be afraid to use a custom type in a case like this. Instead of returning a Result<(String, String), String> where the Err has multiple meanings based on value, just go ahead and make an Enum for your commands. I'll show you this in my final example, along with merging the two command sections (which the exercise is kinda fuzzy on whether it cares which way you do it).

Onto specific things:

You can get an iterator over lines of stdin by .lock().lines(), which can make this simpler, but that's a matter of preference over using read_line. If you are going to use read_line, you might as well reuse your input buffer, since that's the whole point of it taking a buffer as a parameter instead of just returning a String. That way you aren't reallocating each time.

let mut stdin = io::stdin()
let mut input = String::new();
loop {
    input.clear();
    stdin.read_line(&mut input).unwrap();
    // ...
}

dissect_input should take a &str instead of a String, since you only need to view it.

Down in the listing part, you do &command.trim().to_string(), which is unneeded in a few ways. .to_string() takes you from a &str to a String, and then the & brings you right back to a &str, so you could just do command.trim(). Also, you can just bind in the match you already have, like dept => employees.get(dept)

Overall, this is a really good first attempt! It can be hard to first wrap your head around the things that make rust different, like lifetimes and slices, but just getting things working is a great step.

Here's how I might choose to do this exercise.

use std::collections::HashMap;
use std::io;

// required trait for .lines()
use std::io::BufRead;

fn main() {
    let mut employees: HashMap<String, Vec<String>> = HashMap::new();
    let stdin = io::stdin();
    println!("Type 'Add <name> to <department>' to add an employee");
    println!("Type 'List <department>' to list the employees of a department");
    println!("Type 'All' to list all employees by department");
    println!("Type 'Quit' to quit");
    for line in stdin.lock().lines() {
        let input = line.expect("error: unable to read user input");
        match Command::from_input(&input) {
            // or_default is just a convenience, does the same as or_insert_with(Vec::default)
            Some(Command::Add { dept, name }) => employees.entry(dept).or_default().push(name),
            Some(Command::List(dept)) => match employees.get(&dept) {
                Some(names) => {
                    for name in names {
                        println!("{}: {}", dept, name);
                    }
                }
                None => println!("I don't recognize that department!"),
            },
            Some(Command::All) => {
                for (dept, names) in &employees {
                    let mut names = names.clone();
                    names.sort();
                    for name in names {
                        println!("{}: {}", dept, name);
                    }
                }
            }
            Some(Command::Quit) => break,
            // consider using eprintln, which prints to stderr
            None => println!("Input error!"),
        }
    }
    println!("Have a nice day!");
}

enum Command {
    // Using named fields instead of Add(String, String) because dept and name
    // are the same type and could get mixed up.
    Add { dept: String, name: String },
    List(String),
    All,
    Quit,
}

impl Command {
    fn from_input(s: &str) -> Option<Self> {
        let words: Vec<&str> = s.trim().split_whitespace().collect();
        // "Slice destructuring / slice pattern matching" for more info
        match words.as_slice() {
            ["All"] => Some(Command::All),
            ["Quit"] => Some(Command::Quit),
            ["List", dept] => Some(Command::List(dept.to_string())),
            ["Add", name, "to", dept] => Some(Command::Add {
                dept: dept.to_string(),
                name: name.to_string(),
            }),
            _ => None,
        }
    }
}


Kaprekar's constant algorithm in c# I have implemented the kaprekar's constant algorithm in c#, and I want your feedback.

Here is how the algorithm works:

it takes a number of 4 digits (e.g: 5823)
we order that number in ascendent and descendent order, which gives as two numbers (e.g: 8532 and 2358).
we subtract the larger number from the smaller number (e.g: 8532 - 2358 = 6174)
we repeat this operation until the difference is equal to 6174.

Example:

step 1:

 number = 8532
    cpt = 0 // the number of repeatation util the number became equal to 6174.
    1) asc  = 2358
    2) desc = 8532
    3) Difference = 6174
    4) cpt++
    5) Repeating above steps as difference is not equal to number.

step2:

 number = 6174
     1) asc  = 1467
     2) desc = 7641
     3) Difference = 6174
     4) c++
     5) return cpt and stopping here as difference is equal to number.

Here is my Code:

1- recursive method:

   int KaprekarsConstant(int num, int target)
        {
            if (num.ToString().Length != 4)
                throw new ArgumentException();

            var desendingOrder = num.ToString().OrderByDescending(a => a);
            var acendingOrder = num.ToString().OrderBy(a => a);

            int maxPart = int.Parse(new string(desendingOrder.ToArray()));
            int minPart = int.Parse(new string(acendingOrder.ToArray()));

            var diff = maxPart - minPart;
            if (diff == target) return 1;
            return 1+ KaprekarsConstant(diff, target);
        }

2- non-recursive method:

 int KaprekarsConstant2(int num, int target)
            {
                int cpt = 0;
                if (num.ToString().Length != 4)
                    throw new ArgumentException();

                while (true)
                {
                    var desendingOrder = num.ToString().OrderByDescending(a => a);
                    var acendingOrder = num.ToString().OrderBy(a => a);

                    int maxPart = int.Parse(new string(desendingOrder.ToArray()));
                    int minPart = int.Parse(new string(acendingOrder.ToArray()));

                    var diff = maxPart - minPart;
                    cpt++;
                    if (diff == target) return cpt;
                    num = diff;
                }
            }

Code test:

  public static void Main(string[] args)
        {
            Console.WriteLine(KaprekarsConstant(6174, 6174)); //cpt =1
            Console.WriteLine(KaprekarsConstant(3524, 6174)); //cpt =3
            Console.WriteLine(KaprekarsConstant(2324, 6174)); //cpt =3
            Console.WriteLine(KaprekarsConstant(9812, 6174)); //cpt =2
            Console.ReadLine();
        }

Thanks in advance for your help!

6174 is Kaprekar Constant, I don't know why you're using it as an
input argument  while it should be within the method. So, you'll only
need to pass one int argument.
there is no need to call ToString to get the length of the integer.
You can check the integer range instead. So, this num.ToString().Length != 4 can be converted to num >= 1000 && num <= 9999.
using LINQ is fine, but in your case, I don't see a need of that. As you have fixed input length (4 digits), you can simply converted to string, then just do str[index] where the index would be a range of 0 to 3.
why are you using while(true) while you could enforce kaprekar's
constant condition by using while(diff != 6174).
No need for recursive method.

here is a standard version :

int KaprekarsConstant(int num)
{

    // zero == zero
    if (num == 0) { return 0; }

    // if num is not between 1000 and 9999 throw exception
    if(num >= 1000 && num <= 9999) { throw new ArgumentException(); }

    // will be used inside the loop
    var diff = num;

    // number of repeatation
    var numberOfRepeatation = 0;

    do // do it at least once.
    {
        //to array
        var digitsAsc = new int[4];

        for (var x = 0; x < 4; x++)
        {
            digitsAsc[x] = diff % 10;
            diff /= 10;
        }

        // sort the elements in the array to ascendent order
        Array.Sort(digitsAsc);

        //now, digitsAsc array is in ascedent order, we will resorted into descendent order
        var digitsDesc = new int[4];

        for (var x = 0; x < 4; x++)
        {
            digitsDesc[x] = digitsAsc[3 - x];
        }

        // convert them to int
        var asce = int.Parse(string.Join(string.Empty, digitsAsc));

        var desc = int.Parse(string.Join(string.Empty, digitsDesc));

        diff = desc - asce;

        numberOfRepeatation++;

    }
    while (diff != 6174);

    return numberOfRepeatation;
}

if you're into LINQ, here is LINQ version :

int KaprekarsConstant(int num)
{
    // zero == zero
    if (num == 0) { return 0; }

    // if num is not between 1000 and 9999 throw exception
    if(num >= 1000 && num <= 9999) { throw new ArgumentException(); }

    // will be used inside the loop
    var diff = num;

    // number of repeatation
    var numberOfRepeatation = 0;

    do // do it at least once.
    {
        //to ascedent array
        var digitsAsc = Array
            .ConvertAll(diff.ToString().ToCharArray(), x => (int)char.GetNumericValue(x))
            .OrderBy(x=>x)
            .ToArray();

        //now, digitsAsc array is in ascedent order, we will resorted into descendent order by calling Reverse;
        var digitsDesc = digitsAsc.Reverse().ToArray();

        // convert them to string (this is similar to string.Join)
        var asce = digitsAsc
            .Aggregate(new StringBuilder(), (x, y) => x.Append(x.Length == 0 ? string.Empty : string.Empty).Append(y))
            .ToString();

        var desc = digitsDesc
            .Aggregate(new StringBuilder(), (x, y) => x.Append(x.Length == 0 ? string.Empty : string.Empty).Append(y))
            .ToString();

        diff = int.Parse(desc) - int.Parse(asce);

        numberOfRepeatation++;

    }
    while (diff != 6174);

    return numberOfRepeatation;
}


Converting PHP defined constant values to multiple languages We have a large number of defined constants that are used as enum values when storing records. We convert the int values into language for our web portals. This was very straightforward when only dealing with one language, but now are tasked with making the portals support multiple languages.

The functions doing the value to language are easy enough to change and I have a solution, but I was wondering if there wasn't a better way as what I've come up with involves a little too much copy/paste for my taste. What I can't change are how we define the constants/enums.

The language files are written in json as key/value pairs. Language::_($key) handles looking up the proper language files and returning the string.

Another thought I had was to do the lookup based off the constant's value, but I couldn't find a way to pass constant's name (ACTION_ACCOUNT_NEW) to Language::_($key); I believe constants are evaluated before execution. I also don't want to pass the constant's value (1024) to Language::_($key) as I believe that would make a json file that would be very difficult to read.

Would there be a better way of doing this that involves less cut/paste of the defined constant names?

define('ACTION_ACCOUNT_NEW', 1024);
define('ACTION_ACCOUNT_DELETE', 1025);

function ActionToString($action){

    $actions_map = array(
        ACTION_ACCOUNT_NEW => Language::_('ACTION_ACCOUNT_NEW'),
        ACTION_ACCOUNT_DELETE => Language::_('ACTION_ACCOUNT_DELETE')
    );

    return isset($actions_map[$action])
        ? $actions_map[$action]
        : Language::_('Unknown');
}

json

{
    "ACTION_ACCOUNT_NEW": "Account Created",
    "ACTION_ACCOUNT_DELETE": "Account Deleted",

    "Unknown": "Unknown"
}

In all honesty, that seems a bit redundant... But since you can't change the constant values here it goes:

define('ACTION_ACCOUNT_NEW', 1024);
define('ACTION_ACCOUNT_DELETE', 1025);

function ActionToString($action)
{
    $actions_map = array();
    $defined_constants = get_defined_constants(true); // use true to get "user" key

    if (isset($defined_constants['user']))
    {
        $defined_constants = $defined_constants['user'];

        // only process constants that start with "ACTION_"
        $defined_constants = array_intersect_key($defined_constants, array_flip(preg_grep('~^ACTION_~i', array_keys($defined_constants))));

        foreach ($defined_constants as $key => $value)
        {
            $actions_map[$value] = Language::_($key);
        }

        return isset($actions_map[$action]) ? $actions_map[$action] : Language::_('Unknown');
    }

    return Language::_('Unknown');
}

It assumes you only want to parse constants that start with ACTION_, other than that it should work just fine (although, a bit slower due to the looping - still, you could probably fix it with static variables).

One other alternative would be to use class constants but I'm not sure if that option is available to you.


Change functions based on jQuery plugin {options} I'm developing my first legitimate jQuery plugin, and I'm trying to keep it very modular. To do this, I'm writing many individual functions, which is fine, but I need a clean way to determine what functions to fire based on the plugin options provided by the user.

Right now, I'm using a .on('click',...) event with a switch statement. That method is ok and I just have a feeling that there is a better, time-tested method of changing what function is fired based on plugin {options}. A better method that is used by professional JavaScript developers and has been proven to work reliably.

Again, I'm trying to be as modular and professional as possible, but I'm self taught, so there's really no one to say if I'm doing it the "right" way, or the "best" way for future maintainability.

JSFiddle

(function($){

$.fn.showDateTime = function(options) {

    // variables and settings
    var d;
    var $button = $(this);
    var defaults = {
        date: "all"
    };
    var settings = $.extend({}, defaults, options);

    // event handlers
    $button.on('click', function(){
        d = new Date();
        switch(settings.date){
            case "all": showDate();
                        break;
            case "date only": showDateOnly();
                              break;
            case "time only": showTimeOnly();
                              break;
            default: showDate();
        }
    });

    // functions
    function showDate(){
        alert(d.toString());
    }
    function showDateOnly(){
        alert(d.toDateString());
    }
    function showTimeOnly(){
        alert(d.toLocaleTimeString());
    }
};
})(jQuery);

$('#button').showDateTime({date:"date only"});

In my opinion it's a good idea to get rid of the switch case by using better naming for the inputs and by changing the structure of your functions to Object based functions, i.e. if you change your functions to this:

var showDate={
    all:function (){
        alert(d.toString());
    },
    date:function showDateOnly(){
        alert(d.toDateString());
    },
    time:function (){
        alert(d.toLocaleTimeString());
    }
};

and the setting options you get are ['all' , 'date' , 'time']

then you can call these functions like this:

(showDate[settings.date]) ? showDate[settings.date]() : showDate.all();

and get rid of the switch case. DEMO


Multiple longest common subsequence Is the algorithm correct? It seems to work.

print("Multiple sequence longest common subsequence implementation in Python.\nFor example a naive algorithm would perfom c*128^128 elementary operations on a problem set of 128 sequences of length 128 elements.")
print(" That is: ", 128**128, "operations.")

from collections import defaultdict
import sys

def ptable(m, n, table):
   for i in range(m):
       line = ""
       for j in range(n):
           line += str(table[i, j]) + " "
       print(line)

def lcs(x, y):
    table = defaultdict(int)
    n = len(x)
    m = len(y)

    for i in range(n):
        for j in range(m):
            if x[i] == y[j]:
                table[i, j] = table[i-1, j-1]+1
            else:
                table[i, j] = max(table[i-1, j], table[i, j-1])

    return table

def mlcs(strings):
    strings = list(set(strings))
    tables = dict()
    for i in range(1, len(strings)):
        x = strings[i]
        for y in strings[:i]:
            lcsxy = lcs(x, y)
            tables[x,y] = lcsxy

    def rec(indexes):
        for v in indexes.values():
            if v < 0:
                return ""
        same = True
        for i in range(len(strings)-1):
            x = strings[i]
            y = strings[i+1]
            same &= x[indexes[x]] == y[indexes[y]]
        if same:
            x = strings[0]
            c = x[indexes[x]]
            for x in strings:
                indexes[x] -= 1
            return rec(indexes) + c
        else:
            v = -1
            ind = None
            for x in strings:
                indcopy = indexes.copy()
                indcopy[x] -= 1
                icv = valueat(indcopy)
                if icv > v:
                    ind = indcopy
                    v = icv
            indexes = ind
            return rec(indexes)

    def valueat(indexes):
        m = 12777216
        for i in range(1, len(strings)):
            x = strings[i]
            for y in strings[:i]:
                lcsxy = tables[x,y]
                p = lcsxy[indexes[x],indexes[y]]
                m = min(m, p)
        return m

    indexes = dict()
    for x in strings:
        indexes[x] = len(x) - 1
    return rec(indexes)

def check(string, seq):
    i = 0
    j = 0
    while i < len(string) and j < len(seq):
        if string[i] == seq[j]:
            j += 1
        i += 1
    return len(seq) - j

def checkall(strings, seq):
    for x in strings:
        a = check(x, seq)
        if not a == 0:
            print(x, seq, a)
            return False
    return True

strings = []
sigma = ["a", "b", "c", "d"]
#sigma = ["a", "b"]
import random
for i in range(5):
    string = ""
    for j in range(128):
        string += random.choice(sigma)
    strings += [string]

#strings = ["abbab", "ababa", "abbba"]
#strings = ["abab", "baba", "abaa"]
#strings = ["bacda", "abcde", "decac"]
#strings = ["babbabbb", "bbbaabaa", "abbbabab", "abbababa"]
#strings = ["human", "chimpanzee", "woman"]
#strings = ["ababa", "babab", "bbbab"]
#strings = ["ababaaba", "abbabbaa", "aaaaabba"]
#strings = ["aabbbaba", "aaaabbba", "aabbbaab"]
print("Strings:", strings)
l = mlcs(strings)
print("Lcs:", l, checkall(strings, l))`

This test case generation method produces cases where the result is sometimes sub-optimal. Thus there is a flaw in the algorithm.

strings = []
sigma = ["a", "b", "c", "d"]
#sigma = ["a", "b"]
sigmax = ["e", " f", "g"]
import random
Nx = 8
N = 16
M = 5 # Number of strings
x = ""
for i in range(Nx):
    x += random.choice(sigmax)
for i in range(M):
    string = ""
    for j in range(N):
        string += random.choice(sigma)
    indexes = list(range(N))
    random.shuffle(indexes)
    indexes = sorted(indexes[:len(x)])
    for j in range(len(x)):
        string = string[:indexes[j]]+x[j]+string[indexes[j]:]
    strings += [string]

So, back to the drawing board, so to speak.


Finding the first stream of non-repeating elements in Scala (without recursion or side-effects) Here are some examples:

[1, 2, 3, 4, 5] => [1, 2, 3, 4, 5]
[10, 15, 10, 15, 30] => [10, 15]
[1, 2, 3, 4, 1, 5, 6, 7] => [1, 2, 3, 4]

Here's my best (and deeply ugly) non-recursive, side-effect-free solution so far:

x.scanLeft(List[Int]())((B, Term) => Term :: B).drop(1).takeWhile(i => !(i.tail contains i.head)).last.reverse

Minor optimization:

x.tail.scanLeft(List(x.head))((B, Term) => Term :: B).takeWhile(i => !(i.tail contains i.head)).last.reverse

This is different from distinct:

[1, 2, 3, 4, 1, 5, 6, 7] => [1, 2, 3, 4] and not [1, 2, 3, 4, 5, 6, 7]

Also, considering List[_] is a monoid, isn't there a scan that uses the monoid zero?

This is a fold, not a scan. A scan produces something with the same number of elements, and change the elements. A fold produces something new.

def firstDistinct[T](s: Seq[T]) = s.foldLeft(Seq[T]() -> false) {
  case (result @ (_, true), _)           => result
  case ((seq, _), el) if seq contains el => seq -> true
  case ((seq, _), el)                    => (seq :+ el) -> false
}._1


TextBoxes for various number types Is there a way I could somehow have all this classes condensed into one, while keeping the code as clean as possible?

They are all doing essentially the same thing, just for the different number type. I don't know how to "generalize" TryParse() without using switch or if tree which doesn't seem very clean to me.

Base class:

public class TextBoxNum<T> : TextBox where T : IComparable<T>
{
    protected T min;
    protected T max;

    protected Value<T> value;

    public virtual void Update(object sender, EventArgs e)
    {

    }

    protected T Clamp(T input)
    {
        if (input.CompareTo(max) > 0)
        {
            return max;
        }

        if (input.CompareTo(max) < 0)
        {
            return min;
        }

        return input;
    }

    public TextBoxNum(Value<T> value, T min, T max)
    {
        this.value = value;
        this.min = min;
        this.max = max;

        LostFocus += new EventHandler(Update);
    }
}

Int variant:

public class TextBoxInt : TextBoxNum<int>
{
    public override void Update(object sender, EventArgs e)
    {
        int newValue;

        if (int.TryParse(Text, out newValue))
        {
            if (newValue < min || newValue > max)
            {
                newValue = Clamp(newValue);
            }

            value.Set(newValue);
            Text = value.Get().ToString();
        }

        else
        {
            Text = value.Get().ToString();
            Focus();
        }
    }

    public TextBoxInt(Value<int> value, int min, int max) : base(value, min, max)
    {

    }
}

UInt variant:

public class TextBoxUInt : TextBoxNum<uint>
{
    public override void Update(object sender, EventArgs e)
    {
        uint newValue;

        if (uint.TryParse(Text, out newValue))
        {
            if (newValue < min || newValue > max)
            {
                newValue = Clamp(newValue);
            }

            value.Set(newValue);
            Text = value.Get().ToString();
        }

        else
        {
            Text = value.Get().ToString();
            Focus();
        }
    }

    public TextBoxUInt(Value<uint> value, uint min, uint max) : base(value, min, max)
    {

    }
}

ULong variant:

public class TextBoxULong : TextBoxNum<ulong>
{
    public override void Update(object sender, EventArgs e)
    {
        ulong newValue;

        if (ulong.TryParse(Text, out newValue))
        {
            if (newValue < min || newValue > max)
            {
                newValue = Clamp(newValue);
            }

            value.Set(newValue);
            Text = value.Get().ToString();
        }

        else
        {
            Text = value.Get().ToString();
            Focus();
        }
    }

    public TextBoxULong(Value<ulong> value, ulong min, ulong max) : base(value, min, max)
    {

    }
}

Float variant:

public class TextBoxFloat : TextBoxNum<float>
{
    public override void Update(object sender, EventArgs e)
    {
        float newValue;

        if (float.TryParse(Text, out newValue))
        {
            if (newValue < min || newValue > max)
            {
                newValue = Clamp(newValue);
            }

            value.Set(newValue);
            Text = value.Get().ToString();
        }

        else
        {
            Text = value.Get().ToString();
            Focus();
        }
    }

    public TextBoxFloat(Value<float> value, float min, float max) : base(value, min, max)
    {

    }
}

class TextBoxNum

Abbreviation should be avoided when naming a class: Num -> Numeric
The descriptive adjective should also be in front of the noun. It is a numerical textbox, and not a textbox number: NumericTextBoxBase
TextBoxNum and Update should be marked as abstract, unless a generic way of updating the value is provided in TextBoxNum. And by changing it to a abstract class, we should append Base to the end of the class name : NumericTextBoxBase, and its concrete classes can be named as FloatTextBox, DoubleTextBox...

class Value<T>

Without the definition of this class, I can only make some guess about it. We can observe that the value is assigned by a parameter from the constructor. The class itself seems to serve no other purpose than holding the value via void Value<T>.Set(T) and T Value.Get(). I would suggest you to replace them by T or T?, depending on whether or not if you want to enforce valid value is required.

Misc

Avoid try-catch as control-structure much as possible, unless that is your last resort.
Your Clamp method is comparing the value against the same upper boundary, max, twice.

Code:

public class NumericTextBox<T> : TextBox where T : IComparable<T>
{
    protected T min;
    protected T max;
    protected T value;

    public NumericTextBox(T value, T min, T max)
    {
        this.min = min;
        this.max = max;

        // ensure the value is with in valid range
        this.value = Clamp(value);
        // actually you should implement a full property for value and update the Text property in the setter
        this.Text = value.ToString();

        LostFocus += new EventHandler(Update);
    }

    protected virtual void Update(object sender, EventArgs e)
    {
        var type = typeof(T);
        var tryParse = type.GetMethod("TryParse", new[]
        {
            typeof(string), type.MakeByRefType()
        });
        var args = new object[] { Text, null };
        var result = (bool)tryParse.Invoke(null, args);

        if (result)
        {
            value = Clamp((T)args[1]);
            Text = value.ToString();
        }
        else
        {
            Text = value.ToString();
            Focus();
        }
    }
    protected T Clamp(T input)
    {
        if (input.CompareTo(max) > 0)
        {
            return max;
        }

        if (input.CompareTo(min) < 0)
        {
            return min;
        }

        return input;
    }
}


Writing a reusable method in which only some of its parameters are different I have the method below but I am going to have more methods similar to this one. They differ in is the last parameters being passed to the method. The first three parameters will always be there and then in the while loop with && conditions, I am going to use the same params that I am passing in.

I am going to need five or six of these methods. Do you think I can write this method in a more reusable way, or do I have to just copy/paste most of it and change the params and condition for each method?

private string FindFullRowForProvider(string id, string lastName, string firstName, string devSystem)
{
    string fullRow = string.Empty;
    StreamReader streamreader = new StreamReader(importSource);
    char[] delimiter = new char[] { '|' };
    int columnLocation = 0;

    string[] columnheaders = streamreader.ReadLine().Split(delimiter);
    foreach (string columnheader in columnheaders)
    {
        columnHeadertoLocation[columnheader.ToUpper()] = columnLocation;
        columnLocation++;
    }

    while (streamreader.Peek() > 0)
    {
        fullRow = streamreader.ReadLine();
        string[] currentRowValues = fullRow.Split(delimiter);

        string _NPI = currentRowValues[columnHeadertoLocation["ID"]];
        string _LName = currentRowValues[columnHeadertoLocation["LNAME"]];
        string _FName = currentRowValues[columnHeadertoLocation["FNAME"]];
        string _devPKCR = currentRowValues[columnHeadertoLocation["DEV_SYSTEM"]];

        if (npi.Trim().ToUpper() == _NPI.Trim().ToUpper()
            && _LName.Trim().ToUpper() == lastName.Trim().ToUpper()
            && _FName.Trim().ToUpper() == firstName.Trim().ToUpper()
            && _dev.Trim().ToUpper() == devSystem.Trim().ToUpper())
        {
            // woot! we found the row of this guy.
            return fullRow;
        }
    }

    return fullRow;
}

What you're looking for is an Optional Argument. When using optional arguments, you have to supply a default value. So, you just have to check for that default value, and switch your control flow appropriately. It would look something like this. (Note I used empty quotes because String.Empty isn't a constant expression.)

private string FindFullRowForProvider(string id, string lastName, string firstName, string devSystem = "")
{
    //maybe some code here

    if (devSystem == String.Empty)
    {
        //handle that case
    }

    //maybe some more code here

}

But... this is Code Review after all, so let's take a look at what you've done.

private string FindFullRowForProvider(string id, string lastName, string firstName, string devSystem)

I'm making an assumption that id is tied to lastName and firstName,  but it looks like you're missing a class in your design. Creating a Person class would simplify the signature an awful lot. Be wary of passing lots of strings to a method.

private string FindFullRowForProvider(Person somePerson, string devSystem = "")

I like your use of String.Empty. I really do prefer it over empty quotes. I think it makes for nice readable code. Well done.

string fullRow = string.Empty;

Maybe I'm missing something, but I think declaring a character array for delimiter is overkill here.

char[] delimiter = new char[] { '|' };
int columnLocation = 0;

string[] columnheaders = streamreader.ReadLine().Split(delimiter);

I'm not very good at C#, so there may be an even simpler way, but you could certainly just split it like this.

string[] columnheaders = streamreader.ReadLine().Split(new char[] { '|' });

Nice use of brackets and formatting here.

        if (npi.Trim().ToUpper() == _NPI.Trim().ToUpper()
            && _LName.Trim().ToUpper() == lastName.Trim().ToUpper()
            && _FName.Trim().ToUpper() == firstName.Trim().ToUpper()
            && _dev.Trim().ToUpper() == devSystem.Trim().ToUpper())
        {
            // woot! we found the row of this guy.
            return fullRow;
        }


Displaying total cost of prices from a table I have this JavaScript function which gets prices from a table, adds them up, and outputs the total cost. I want to refactor this to make it as small as possible.

function getTotalPrice(do_request)
{
console.log('Get total price');
var prices_array = new Array(); // Where our prices are held

// For each .total_item_price - a <td> within my table.
$(".total_item_price").each(function(){
    var text = $(this).text();                   // Get the value
    var prices = text.substring(1, text.length); // Format it into a string
    prices_array.push(prices);                   // Push it onto our array
});

var result = eval(0);
//  Add up our array
for(i = 0; i < prices_array.length; i++)
{
    temp = eval(prices_array[i]);
    result += temp;
}
// Round up our result to 2 Decimal Places
result = Math.round(result*100)/100;
// Output/Return our result
if (do_request == null)
{
    // We want to add our shipping Price and Display the total
    // Get the Shipping Price
    //var shipping_price = $(".shipping_price").html();
    //shipping_price = shipping_price.substring(1, shipping_price.length);
    if ($(".tbl_row").length == 1) // If only 1 row exists (How do I do this?)
    {
         var size = $(".size").html();
         if((size == "A5") && ($('input[name="qty"]').val() == '1'))
         {
            console.log(' - A5 - ');
            $('.shipping_price').html('£0.67'); // Show Shipping Price (A5)
         }
         else if((size == 'A4') && ($('input[name="qty"]').val() == '1'))
         {
            console.log(' - A4 - ');
            console.log($('input[name="qty"]').val());
            $('.shipping_price').html('£0.87'); // Show Shipping Price (A4)
         }
         else
         {
            console.log('More then 1 Qty');
            $('.shipping_price').html('£0.97'); // Show Shipping Price (A4)
         }
    }
    else  // More then 1 row exists, so the price will be £0.97
    {
        console.log('false, More rows exists');
        $('.shipping_price').html('£0.97') // Show Shipping Price (A4)
    }

    var shipping_price = $(".shipping_price").html();
    shipping_price = shipping_price.substring(1, shipping_price.length);

    // Add em
    //result += eval(shipping_price);
    result += eval(shipping_price);

    // Round our result to 2 decimal places
    var result=Math.round(result*100)/100;

    // Update & Display the Result
    $('.total_order_price').html("<b>£" + result + "</b>");
}
else
{
    // Otherwise we just want the total price and return it.
    return result;
}
}

First, I would switch from using eval to parseFloat, since it appears that eval provides no extra features and allows arbitrary code to be executed unless you just happen to have values other then numbers in total_item_price tds.

Second, since you are performing no extra processing with prices_array, it would be better to skip the array altogether and combine:

 var prices_array = new Array(); // Where our prices are held
 // For each .total_item_price - a <td> within my table.
 $(".total_item_price").each(function(){
     var text = $(this).text();                   // Get the value
     var prices = text.substring(1, text.length); // Format it into a string
     prices_array.push(prices);                   // Push it onto our array
 });

 var result = eval(0);
 //  Add up our array
 for(i = 0; i < prices_array.length; i++)
 {
     temp = eval(prices_array[i]);
     result += temp;
 }

into (with result renamed as totalSum for readability):

 var totalSum = 0;
 // For each .total_item_price - a <td> within my table.
 $(".total_item_price").each(function(){
     var text = $(this).text();                   // Get the value
     var price  = text.substring(1, text.length); // Format it into a string
     price = parseFloat(price);
     totalSum += price;
 });


pythonic longest repetition I put together this code to for the Udacity cs101 exam. It passes the test, but I feel their must be a more elegant way to manage this problem. Some help from a module (like itertools).

Question 8: Longest Repetition

Define a procedure, longest_repetition, that takes as input a
 list, and returns the element in the list that has the most
 consecutive repetitions. If there are multiple elements that
 have the same number of longest repetitions, the result should
 be the one that appears first. If the input list is empty,
 it should return None.

def longest_repetition(alist):
    alist.reverse()
    largest = []
    contender = []
    if not alist:
        return None
    for element in alist:
        if element in contender:
            contender.append(element)
            if len(contender) >= len(largest):
                largest = contender
        else:
            contender = []
            contender.append(element)

    if not largest:
        return contender[0]
    else:
        return largest[0]

#For example,

print longest_repetition([1, 2, 2, 3, 3, 3, 2, 2, 1])
# 3

print longest_repetition(['a', 'b', 'b', 'b', 'c', 'd', 'd', 'd'])
# b

print longest_repetition([1,2,3,4,5])
# 1

print longest_repetition([])
# None

First, I'll comment on your code, and then I'll discuss a more "Pythonic" way to do it using features in the Python standard library.

There's no docstring. What does the function do?
You start out by reversing the list. This is a destructive operation that changes the list. The caller might be surprised to find that their list has changed after they called longest_repetition! You could write alist = reversed(alist) instead, but it would be even better if you didn't reverse the list at all. By changing the >= test to > you can ensure that the earliest longest repetition is returned, while still iterating forwards over the input.
You build up a list contender that contains the most recent series of repetitions in the input. This is wasteful: as contender gets longer, the operation element in contender takes longer. Since you know that this list consists only of repetitions of an element, why not just keep one instance of the element and a count of how many repetitions there have been?
You only update largest when you found a repetition. That means that if there are no consecutive repetitions in the input, largest will never be updated. You work around this using an if statement at the end to decide what to return. But if you always updated largest (whether you found a repetition or not) then you wouldn't need the if statement.
By suitable choice of initial values for your variables, you can avoid the if not alist special case. And that would allow your function to work for any Python iterable, not just for lists.

So let's apply all those improvements:

def longest_repetition(iterable):
    """
    Return the item with the most consecutive repetitions in `iterable`.
    If there are multiple such items, return the first one.
    If `iterable` is empty, return `None`.
    """
    longest_element = current_element = None
    longest_repeats = current_repeats = 0
    for element in iterable:
        if current_element == element:
            current_repeats += 1
        else:
            current_element = element
            current_repeats = 1
        if current_repeats > longest_repeats:
            longest_repeats = current_repeats
            longest_element = current_element
    return longest_element

So is there a more "Pythonic" way to implement this? Well, you could use itertools.groupby from the Python standard library. Perhaps like this:

from itertools import groupby

def longest_repetition(iterable):
    """
    Return the item with the most consecutive repetitions in `iterable`.
    If there are multiple such items, return the first one.
    If `iterable` is empty, return `None`.
    """
    try:
        return max((sum(1 for _ in group), -i, item)
                   for i, (item, group) in enumerate(groupby(iterable)))[2]
    except ValueError:
        return None

However, this code is not exactly clear and easy to understand, so I don't think it's actually an improvement over the plain and simple implementation I gave above.