connect in a background until success We have some required services down temporarily when our server start. So we need some reconnection logic for them until they are finally up.
There is a requirement to have also syncronious way for that.
Here is my generic implementation for that via a single background thread:

public class RunUntilSuccess {

private static final Logger log = LoggerFactory.getLogger(RunUntilSuccess.class);

private final String processName;
private final Task task;
private final int maxAttempts;
private final int interval;
private final TimeUnit unit;
private final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
private AtomicInteger count = new AtomicInteger(0);

public RunUntilSuccess(String processName, Callable<Boolean> task, int interval, TimeUnit unit) {
    this(processName, task, interval, unit, -1);
}

public RunUntilSuccess(String processName, Callable<Boolean> callable, int interval, TimeUnit unit, int maxAttempts) {
    if (callable == null) {
        throw new IllegalArgumentException("Callable cannot be null");
    }
    this.processName = processName;
    this.task = new Task(callable);
    this.interval = Math.max(0, interval);
    if (unit == null) {
        throw new IllegalArgumentException("Unit cannot be null");
    }
    this.unit = unit;
    this.maxAttempts = (maxAttempts > 0) ? maxAttempts : -1;

    start();
}

private void start() {
    log.debug("Starting task execution. " + toString());
    executor.scheduleWithFixedDelay(task, 0, interval, unit);
}

/**
 * Wait until success
 */
public void await() throws InterruptedException {
    while (!executor.awaitTermination(interval, unit)) { }
}

public boolean await(long timeout, TimeUnit unit) throws InterruptedException {
    return executor.awaitTermination(timeout, unit);
}

private boolean isAttemptsLimitReached(int attempt){
    return (maxAttempts > 0) && (attempt >= maxAttempts);
}

@Override
public String toString() {
    final StringBuilder sb = new StringBuilder();
    sb.append(RunUntilSuccess.class.getSimpleName());
    sb.append("{processName='").append(processName).append('\'');
    sb.append(", interval=").append(interval);
    sb.append(", unit=").append(unit);
    sb.append(", maxAttempts=").append(maxAttempts);
    sb.append('}');
    return sb.toString();
}

private class Task implements Runnable {
    private final Callable<Boolean> task;

    public Task(Callable<Boolean> task) {
        this.task = task;
    }

    @Override
    public void run() {
        if (isAttemptsLimitReached(count.getAndIncrement())) {
            log.debug("Task execution finished unsuccessfully after " + count + " attempts.");
            executor.shutdown();
        } else {
            log.debug("Attempt #" + count);
            try {
                if (Boolean.TRUE.equals(task.call())) {
                    log.debug("Attempt #" + count + " was successful. Task execution finished.");
                    executor.shutdown();
                } else {
                    throw new FalseExecution();
                }

            } catch (Exception e) {
                log.error("Attempt #" + count + " failed due to:", e);
            }
        }
    }
}

public static class FalseExecution extends RuntimeException {

}

usage:

Callable<Boolean> connector = new Callable<Boolean>() {
        @Override
        public Boolean call() throws Exception {
            // connect to ...
            return true;
        }
    };

    RunUntilSuccess initializer = new RunUntilSuccess("name", connector, reconnectTimeoutSeconds, TimeUnit.SECONDS);
            // initializer.await() after it for sync way

Is it a common task and how generic this code?
Thanks!

Your use of executor.awaitTermination() seems odd to me. I would instead have used a CountdownLatch to trigger success in a blocked thread. But I also agree with @Craig P. Motlin that a Future makes sense.

Besides that specific comment, you might consider some asynchronous notification technique for the dependency to declare that it's alive. That might be overkill for your scenario, but it's worked for me. Jini service registry, JMS messages, Hazelcast, plain UDP multicast, etc.


C program circular array rotation I'm trying to solve a Hackerrank problem where you perform k array[n] rotations, and at the end, ask for m and print array[m]. My program is probably correct (it runs perfectly for most of tests, but some of them terminate due to timeout), but is inefficient and I don't know how to improve it.

#include <math.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <assert.h>
#include <limits.h>
#include <stdbool.h>

int main(){
    int n;
    int k;
    int q;
    scanf("%d %d %d",&n,&k,&q);
    int *a = malloc(sizeof(int) * n);
    int *b = malloc(sizeof(int) * n);
    for(int a_i = 0; a_i < n; a_i++){
       scanf("%d",&a[a_i]);
    }
    for(int i = 0; i < k; i++){
        b[0] = a[n-1];
        for(int a_i = 1; a_i < n; a_i++){
        b[a_i] = a[a_i-1];
        }
        for(int a_i = 0; a_i < n; a_i++) a[a_i] = b[a_i];
    }
    for(int a0 = 0; a0 < q; a0++){
    int m;
    scanf("%d",&m);
    printf("%d\n", b[m]);
    }
    return 0;
}

Do not do actual rotation.

k rotations (from your code I understand that all rotations are to the right by 1) map an i'th element to an (i + k) % n position. At the time of printing, do some math to figure out which element was mapped to the position m.

The math is trivial, and I intentionally do not spell it out.


Moving to other game states with ScreenManager I'm creating a game in C#, and I've created a ScreenManager so I can call

ScreenManager.MoveToScreen(typeof(ScreenClassHere))

to move to other game states. However, I feel that my method of creating an instance of the class is... not very optimal. See for yourself.

    public static void MoveToScreen(Type s)
    {
        if (currentscreen != null) currentscreen.Remove();

        currentscreen = (Screen)s.GetConstructor(System.Type.EmptyTypes).Invoke(null);
        currentscreen.Init();
    }

Does anyone know if there's a better way to create an instance of a certain class from a Type?

You could use generics

public static void MoveToScreen<T>() where T : Screen, new()
{
    if (currentscreen != null) currentscreen.Remove();

    currentscreen = new T();
    currentscreen.Init();
}

In this case, you'd have to call the method as

ScreenManager.MoveToScreen<ScreenClassHere>();


Locating numbers for a given name I'm finding names then going to the corresponding column to get the number, then search for the number in column A. If I find the number then go to the corresponding column, copy and paste it to a different worksheet.

How can I make this code faster and more efficient? I found several examples in here, someone suggested to use Variants as well as UBound() and LBound(). I used it but the performance is still the same: It's taking more than 5 mins to loop through 1 million rows.

 Dim wsheet1 as Worksheet, wsheet2 as Worksheet, lastrow as long, correspondingValue1 as Object, correspondingOffset as Object, searchNum as Object, data1, data1Range as Range, x as Long, y as Long

   Set wsheet1 = ActiveWorkbook.Worksheets("Sheet1")
   Set wsheet2 = ActiveWorkbook.Worksheets("Sheet2")
   Set data1Range =  wsheet2.Range(wsheet2.Range("C1"), wsheet2.Cells(Rows.Count, 1).End(xlUp))
   data1 = data1Range.Value

       For i = 1 To UBound(data1, 1)
            If wsheet2.Cells(i, 3).Value = "Mike" Or wsheet2.Cells(i, 3) = "Sam" Or wsheet2.Cells(i, 3) = "George" Or wsheet2.Cells(i, 3) = "Abrahim" Then
              Set correspondingValue1 = wsheet2.Cells(i, 3)
              Set correspondingOffset = corespondingValue1.Offset(columnoffset:=7)

                If InStr(1, xoffset, "1.") <> 0 Then
                    With wsheet2.Range("a1:a" & lastrow)
                    Set searchNum = .Find(wsheet2.Range("J" & i).Value, LookIn:=xlValues)

               If Not searchNum Is Nothing Then
                    firstAddress = searchNum.Address

                  Do
                     searchNum.Offset(columnoffset:=3).Copy
                       wsheet1.Range("B" & x).PasteSpecial
                  Loop While Not searchNum Is Nothing And searchNum.Address <> firstAddress
             x = x + 1
            End If
        End With
    End If
    End If

I've done my best to get you pointed in the right direction, but your code is difficult to follow as is. Here is what I have:

Sub Test()
    Dim wsheet1 As Worksheet
    Set wsheet1 = ActiveWorkbook.Worksheets("Sheet1")

    Dim wsheet2 As Worksheet
    Set wsheet2 = ActiveWorkbook.Worksheets("Sheet2")

    ' Use the Range object, and not the 'Object' object
    Dim data1Range As Range
    Set data1Range = wsheet2.Range(wsheet2.Range("C1"), wsheet2.Cells(Rows.Count, 1).End(xlUp))

    ' I prefer clearer variable names
    Dim InputData As Variant

    ' I am making the somewhat dangerous assumption here that the only data on this sheet is the data you need in the array.
    InputData = wsheet2.UsedRange.value

    ' I am creating this boolean because I am a little too lazy to do a fancier function approach.
    ' This is purely to get a satisfied condition from the Select Case statement, and as a result to skip the corresponding conditional block.
    Dim SatisfiesConditions As Boolean

    ' I dont know what kind of value you're retrieving from the corresponding cells, so I am using variants as a temporary type.
    Dim MatchedValue As Variant
    Dim OffsetValue As Variant

    Dim SearchNum As Range
    Dim FirstAddress As String
    Dim x As Long

    ' I assume here that your data has headers, so I loop from the lowest bound (first row) plus 1 (offset for header row) to the maximum row
    For i = LBound(InputData, 1) + 1 To UBound(data1, 1)
        ' To use an array, reference the array index and not the worksheet index
        ' Additionally, use 'Select Case' for multiple 'Or' conditions.

        SatisfiesCondition = True

        Select Case InputData(i, 3)
            Case "Mike"
            Case "Sam"
            Case "George"
            Case "Abrahim"
            Case Else
                SatisfiesCondition = False
        End Select

        If SatisfiesCondition Then
            MatchedValue = InputData(i, 3)

            ' Since the offset was 7, 3 + 7 = 10
            OffsetValue = InputData(i, 10)

            ' From here down, it is difficult to understand what you are doing. xOffset isn't declared (or assigned)
            If InStr(1, xoffset, "1.") <> 0 Then
                With wsheet2.Range("a1:a" & lastrow)
                    Set SearchNum = .Find(wsheet2.Range("J" & i).value, LookIn:=xlValues)

                    If Not SearchNum Is Nothing Then
                        FirstAddress = SearchNum.Address

                        ' This loop should be infinite since searchNum will always be assigned to the same exact
                        ' address, and as a result the condition will always be satisfied.
                        Do
                            SearchNum.Offset(columnoffset:=3).Copy
                            wsheet1.Range("B" & x).PasteSpecial
                        Loop While Not SearchNum Is Nothing And SearchNum.Address <> FirstAddress
                        x = x + 1
                    End If
                End With
            End If
        End If
    Next
End Sub

I noticed a few points where you code looks as though it shouldnt actually work. You also had a mix of variables declared and never used, and variables used but never declared.

The Big Stuff

First, multiple Dim statements are always preferable to a single Dim statement for multiple variables

Dim Foo as Long
Dim Bar as String
Dim Baz as Worksheet

Is better than

Dim Foo as Long, Bar as String, Baz as Worksheet

It is also best to declare variables as close to first use as possible (outside of loops though). You'll notice that I implemented this in my refactoring of your code.

Option Explicit

I highly encourage the use of Option Explicit within your code. This will give you a compile-time error any time you try to compile your code when a variable is used but undeclared. This helps with things like:

Dim Fooo as String
Fooo = "SomeString"

...many lines later...

If Foo = "SomeString" Then
    ' Never is satisfied since Foo is not the same as Fooo
End If

To make Option Explicit easy, do this:
    Alt + T
    Alt + O
    Editor Tab
    Make sure 'Require Variable Declaration' is checked

Indentation

Make sure your code is properly indented before posting it. If you did, you would've noticed that you were missing the Next at the end of your For block (which prevents the code from compiling). You would've also noticed the lack of Sub and End Sub.

Select Case vs. If...Or...Or...Or...Or...

Any time you are doing multiple "If it is this, or that, or..." conditionals, you should use a Select Case statement. There is a slight performance increase, but it is also cleaner and easier to read.

Additionally, it makes it easy to edit the conditions as needed.

Select Case works by taking an input variable, and checking the condition against Case's. If the condition matches, it runs the code inside the Case block, otherwise it goes to Case Else or End Select in the absence of Case Else.

For example:

Dim Foo as String
Foo = "Apple"

Select Case Foo
Case "Apple"
    msgbox "Foo is an Apple!"
Case "Pear"
    msgbox 'Foo is a Pear!'
Case "Orange"
    msgbox "Foo is an Orange!"
Case Else
    msgbox "Foo is an " & Foo & "!"
End Select

' Outputs a messagebox that says "Foo is an Apple!"

Dim Foo as String
Foo = "Banana"

Select Case Foo
Case "Apple"
    msgbox "Foo is an Apple!"
Case "Pear"
    msgbox 'Foo is a Pear!'
Case "Orange"
    msgbox "Foo is an Orange!"
Case Else
    msgbox "Foo is an " & Foo & "!"
End Select

' Outputs a messagebox that says "Foo is a Banana!"

The beauty of Select Case is that you can have code within the Case Blocks or you can leave them empty and only have code in the Case Else block.

Arrays

This is the meat of what you are looking for. From your code, it looks like you misunderstood how to use arrays. Basically, arrays improve performance when the computations are done in the array (this is because they are being done in-memory versus in-worksheet).

When using an array for your purposes I would suggest setting the array equal to the entire range with your data in it. So for example, if you have a table on worksheet Sheet in `Range("A1:Z100") your array would be:

Dim SomeArray as Variant
SomeArray = ThisWorkbook.Worksheets("Sheet1").Range("A1:Z100").Value

This will return a 1-Based array that you can then loop through. Now, for example, if Column "C" should equal "B" * "A" you can do this:

Dim i as Long
For i = LBound(SomeArray, 1) + 1 to Ubound(SomeArray, 1)
    If IsNumeric(SomeArray(i, 1)) and IsNumeric(SomeArray(i, 2)) Then
        SomeArray(i, 3) = SomeArray(i, 2) * SomeArray(i, 3)
    End If
Next

Then we can quickly output it to the worksheet when we are done:

ThisWorkbook.Sheets("Sheet1").Range("A1:Z100").Value = SomeArray

Voila! We now have completed our computations much faster than if we had done the same thing on the worksheet.

I would recommend practicing with arrays and spending some time understanding what they do, how they work, and how to loop through them. They are very powerful when used well.

Rubberduck

If you haven't heard of it before, there is a tool called "RubberDuck" that can help tremendously with refining your code. It has inspections that can catch a lot of the common mistakes. It also improves the Project Explorer window immensely. Check it out here: http://rubberduckvba.com/

Locals, Immediate, and Watch Windows

If you aren't using these windows already you definitely should be. They make debugging much easier, and it also improves your ability to step through your code. You can find them in the 'View' menu of the IDE.

Best of luck!


Read matrix from file, the C++11/14 way I am trying to get acquainted with C++11/14, so please tell me if the code below could be written in a more "modern" way. Or, of course, if it could be improved in any way.

The function buildInput reads the content of a file and builds a matrix with it.

std::vector<std::vector<int>> buildInput(std::string fileName) {
    auto rows = 0, cols = 0;
    auto objectsCount = 0;

    std::ifstream in(fileName);

    // Throw exceptions in case any i/o operation fails.
    in.exceptions(std::ifstream::failbit | std::ifstream::badbit);

    in >> rows >> cols >> objectsCount;
    std::vector<std::vector<int>> input(rows, std::vector<int>(cols, 0));

    for(int i = 0; i < objectsCount; ++i) {
        auto type = 0, row = 0, col = 0;
        in >> type >> row >> col;
        input[row][col] = type;
    }

    return input;
}

The function builds the input for a Sokoban game from a file. Basically, it builds a "map" (a matrix) with several types of objects placed on it.

The file has the following format:

rows_count columns_count objects_count
[object_type object_row_number object_column_number](objects_count times)

For example, for the following file content:

4 4 6
1 0 3
2 1 2
3 1 3
1 2 3
4 3 1
1 3 3

the code would build the matrix

0 0 0 1
0 0 2 3
0 0 0 1
0 4 0 1

I see some things that may help you improve your code.

Use the required #includes

The code uses std::vector which means that it should #include <vector>.  It was not difficult to infer, but it helps reviewers if the code is complete.  The full set of required includes appears to be this:

#include <iostream>
#include <fstream>
#include <vector>

Think of the user

The provided example input requires 21 numbers so that the computer can create a matrix containing just 16.  There is a lot of redundancy there!  It seems to me that simply having your example output be the input format would make a lot more sense and require less space, less typing and less interpretation.

Sanitize user input better

If I change the last line of the sample input to 1 9 9 it is asking the code to initialize a location that is outside the bounds of the vector and on my machine the computer segfaults and the program dies.  Some bounds checking would be prudent.

Pass a stream rather than a filename

The current design is rather inflexible in that it can only use an actual file as input.  I'd recommend making it more generic and accepting a std::istream & as a parameter rather than a file name.

Use a better data structure

The choice of a vector of vectors for a matrix is not a very good one because unlike a matrix, there is nothing to assure that all of the vectors are the same size.  I'd recommend creating your own custom object which hides the messy details of the actual storage (it might be a std::array or a single std::vector) but could do bounds checking and retrieval using the same coordinate system.  An example is shown below.

Use a friend extractor instead of a standalone function

Rather than having a separate function, it would be nicer to be able to write code something like this:

SokobanBoard board;
std::cin >> board;

Alternatively,

SokobanBoard board;
ifstream in(fileName);
in >> board;

To do this, we could write an extractor.  Here's a sketch of an example:

class SokobanBoard {
public:
    SokobanBoard(std::size_t rows, std::size_t cols);
    int at(std::size_t x, std::size_t y) const;

    friend std::ostream& operator<<(std::ostream& out, const SokobanBoard &sb);
    friend std::istream& operator>>(std::istream& in, SokobanBoard &sb);
private:
    std::size_t m_height;
    std::size_t m_width;
    std::vector<int> m_board;
};

A word about addressing individual squares

A question in the comments was whether doing the mathematics (multiplication) might be longer than the addressing of a vector of vectors.  On my 64-bit Linux machine using gcc 6.2.1 with -O3 optimization, I coded two different versions.

First, here is the test source code:

#include <vector>
#include <iostream>

class Test1 {
public:
    Test1(int rows, int cols) :
        vec(rows)
    {
        for (auto &row: vec) {
            row.reserve(cols);
        }
    }
    int fetch(int x, int y) const;
private:
    std::vector<std::vector<int>> vec;
};

class Test2 {
public:
    Test2(int rows, int cols) :
        height{rows},
        width{cols},
        vec(height * width)
    {}
    int fetch(int x, int y) const;
private:
    const size_t height;
    const size_t width;
    std::vector<int> vec;
};

int Test1::fetch(int x, int y) const { return vec[x][y]; }
int Test2::fetch(int x, int y) const { return vec[x+y*height]; }

int main() {
    std::size_t rows, cols;
    std::cin >> rows >> cols;
    Test1 test1(rows, cols);
    Test2 test2(rows, cols);
    std::size_t x, y;
    std::cin >> x >> y;
    int m = test1.fetch(x, y);
    int n = test2.fetch(x, y);
    std::cout << "m = " << m << ", and n = " << n << "\n";
}

Note that this is a poor program lacking all error checking, but the purpose was only to extract the assembly language for the two versions of fetch.  First, this version

int Test1::fetch(int x, int y) const { return vec[x][y]; }

produced this assembly code

0000000000000000 <Test1::fetch(int, int) const>:
Test1::fetch(int, int) const():
   0:   48 63 f6                movslq %esi,%rsi
   3:   48 8b 0f                mov    (%rdi),%rcx
   6:   48 63 d2                movslq %edx,%rdx
   9:   48 8d 04 76             lea    (%rsi,%rsi,2),%rax
   d:   48 8d 04 c1             lea    (%rcx,%rax,8),%rax
  11:   48 8b 00                mov    (%rax),%rax
  14:   8b 04 90                mov    (%rax,%rdx,4),%eax
  17:   c3                      retq

This version

int Test2::fetch(int x, int y) const { return vec[x+y*height]; }

produced this assembly code

0000000000000020 <Test2::fetch(int, int) const>:
Test2::fetch(int, int) const():
  20:   48 63 d2                movslq %edx,%rdx
  23:   48 8b 47 10             mov    0x10(%rdi),%rax
  27:   48 63 f6                movslq %esi,%rsi
  2a:   48 0f af 17             imul   (%rdi),%rdx
  2e:   48 01 d6                add    %rdx,%rsi
  31:   8b 04 b0                mov    (%rax,%rsi,4),%eax
  34:   c3                      retq

I was not able to detect any timing differences with the testing I did, but we can see that the difference is that there are 5 memory fetches for the vector of vectors version, versus three for the multiplying version.  If the structure happens to be in the cache (likely the case in my testing where I was using small dimensions), they are both about the same duration.  If there is a cache miss, the multiplying version will likely be faster.

I'd always encourage you to measure it on your own machine with your own data to determine which is better for your purposes.


Java number formatting in accountant style I have written a string number formatter. I would like to know if this can be improved and any suggestions with how it's done.

The purpose of this function is to convert a double that has been converted to a string, using String.valueOf to a accounting style formatted number.

/**
     * this function formats a number string to show commas and an arbitrary amount
     * of decimal places
     * @param string the number String
     * @param decimal the decimal precision
     * @return
     */
private String formatNumberString(String string, int decimal) {

    char[] num = string.toCharArray();
    char[] reverse = reverse(num);
    int cnt = 0;
    List<Character> chars = new ArrayList();
    boolean fd = false;
    int fCnt = 0;

    if (contains(reverse, '.')) {
        for (char a : reverse) {
            if (!fd) {
                chars.add(a);
            } else {
                if (fCnt == 3) {
                    chars.add(',');
                    fCnt = 0;
                }
                chars.add(a);

                fCnt++;
            }

            if (a == '.') {
                fd = true;
            }

        }
    } else {
        for (char a : reverse) {
            if (fCnt == 3) {
                chars.add(',');
                fCnt = 0;
            }
            chars.add(a);
            fCnt++;
        }

    }
    char[] fin = new char[chars.size()];
    int cntr = 0;
    for (Character ch : chars) {
        fin[cntr++] = ch;
    }

    String finall = String.valueOf(reverse(fin));
    if (decimal == 0) {
        int dot = finall.indexOf(".");
        finall = finall.substring(0, dot);
    }

    if (finall.equals("0") || finall.equals("0.0") || finall.equals("0.00")) {
        finall = " - ";
    }

    return finall;
}

My contains function checks to see the array contains a decimal

/**
     * this function checks to see if a decimal is present in char array
     * @param ch
     * @param c
     * @return
     */
    public boolean contains(char[] ch, char c) {
        for (char a : ch) {
            if (a == c) {
                return true;
            }
        }
        return false;
    }

My reverse function is simple it reverses the char array

 public char[] reverse(char[] ch) {
        char[] temp = new char[ch.length];
        int cnt = 0;
        for (int i = ch.length - 1; i >= 0; i--) {
            temp[cnt++] = ch[i];
        }
        return temp;

    }

If you come back in a few months to that code you will likely have a problem at figuring out what your code is doing. This is due to the abbreviations you used to name your things like

int cnt = 0;

boolean fd = false;
int fCnt = 0;

for e.g you just won't know what fd should represent.

Naming things is very improtant in the programming field on the other side it is pretty hard as well. Try to give your things meaningful names stating its purpose. By reading the name later you will grasp easier what your code is about.

char[] fin = new char[chars.size()];
int cntr = 0;
for (Character ch : chars) {
    fin[cntr++] = ch;
}

It would be better to just use the toArray() method specified by the List<E>.

For reversing the array you could use ArrayUtils.reverse() see also https://stackoverflow.com/a/2138004/2655508 which lives in Commons.Lang

You contains() and reverse() method are publicbut you aren't doing any parameter validation. If the passed in char[] ch would be null you would get an NullPointerException.


Better way to use a Condition (CancellationToken) Is there a better way to write this, as I am using the condition of the Cancellation Token to tell if it should use a byte[] from another thread or not.

        SharpDX.Windows.RenderLoop.Run(form, () =>
        {
            if (CTS.IsCancellationRequested)
                form.Dispose();

            if (Queue.TryTake(out TextureData, 300))
            {
                Stoptimer = new Stopwatch();
                Stoptimer.Start();

                device.BeginScene();
                sprite.Begin(SpriteFlags.None);
                try
                {
                    using (var surface = texture.GetSurfaceLevel(0))
                    {
                        if (!CTS.IsCancellationRequested)
                            Surface.FromFileInMemory(surface, TextureData, SharpDX.Direct3D9.Filter.None, 0);
                    }
                    sprite.Draw(texture, new ColorBGRA(0xffffffff));
                }

                catch (Exception ex)
                {
                    MessageBox.Show(ex.Message, "Rendering: Texture");
                }

                sprite.End();
                device.EndScene();
                device.Present();
                Stoptimer.Stop();
                Console.WriteLine(Stoptimer.ElapsedTicks);
            }
        });

The important place is here:

        if (!CTS.IsCancellationRequested)
            Surface.FromFileInMemory(surface, TextureData, SharpDX.Direct3D9.Filter.None, 0);

As I need to recheck this, as the Cancellation can happen After it has taken the data from the Queue.

But I wonder, is there a better way to write it?

As you can see, I use the CTS condition 2 times, and it would be nice if I could only use it once to control the flow or something.

EDIT

I think this is what you are looking for, it will only check once inside the using block, if you want to dispose the form if that first Queue.TryTake(out TextureData, 300)) is false then add to the else of that statement.

if you really want to test it at the last second you could do it like this

 SharpDX.Windows.RenderLoop.Run(form, () =>
    {
        if (Queue.TryTake(out TextureData, 300))
        {
            Stoptimer = new Stopwatch();
            Stoptimer.Start();

            device.BeginScene();
            sprite.Begin(SpriteFlags.None);
            try
            {
                using (var surface = texture.GetSurfaceLevel(0))
                {
                    if (!CTS.IsCancellationRequested)
                    {
                        Surface.FromFileInMemory(surface, TextureData, SharpDX.Direct3D9.Filter.None, 0);
                    }
                    Else
                    {

                        // I imagine if you close the form that you want this stuff to
                        //happen as well
                        sprite.End();
                        device.EndScene();
                        device.Present();
                        Stoptimer.Stop();
                        Console.WriteLine(Stoptimer.ElapsedTicks);
                        form.Dispose();
                    }
                }
                sprite.Draw(texture, new ColorBGRA(0xffffffff));
            }

            catch (Exception ex)
            {
                MessageBox.Show(ex.Message, "Rendering: Texture");
            }

            sprite.End();
            device.EndScene();
            device.Present();
            Stoptimer.Stop();
            Console.WriteLine(Stoptimer.ElapsedTicks);
        }
        //Optional for Disposing of the form if Queue.TryTake(out TextureData, 300))
        // is false
        Else
        {
            form.Dispose();
        }
    });


CarTalk's Homophones Puzzler: A Programmatic Solution The puzzler:

  This was sent in by a fellow named Dan O'Leary. He came upon a common
  one-syllable, five-letter word recently that has the following unique
  property. When you remove the first letter, the remaining letters form
  a homophone of the original word, that is a word that sounds exactly
  the same. Replace the first letter, that is, put it back and remove
  the second letter and the result is yet another homophone of the
  original word. And the question is, what's the word?

  Now I'm going to give you an example that doesn't work. Let's look at
  the five-letter word, 'wrack.' W-R-A-C-K, you know like to 'wrack with
  pain.' If I remove the first letter, I am left with a four-letter
  word, 'R-A-C-K.' As in, 'Holy cow, did you see the rack on that buck!
  It must have been a nine-pointer!' It's a perfect homophone. If you
  put the 'w' back, and remove the 'r,' instead, you're left with the
  word, 'wack,' which is a real word, it's just not a homophone of the
  other two words.

I've used CMU Pronouncing Dictionary and modified it (manually) to keep alphabetical strings only.

My code:

def read_dict(file_path):
    """Reads 'CMU Pronouncing Dictionary' text file.

       returns: dict mapping from a word to a string describing
       its pronounciation

       file_path: str
       """
    fin = open(file_path)
    pron_dict = {}

    for line in fin:
        index = line.find(' ')  # e.g. "AA  EY2 EY1"
        word = line[:index]
        pron = line[index+2:]
        pron_dict[word] = pron

    return pron_dict

def are_homophones(pron_dict, word1, word2, word3):
    """Returns whether 3 words are homophones.

       pron_dict: dict
       """

    for word in [word1, word2, word3]:
        if not word in pron_dict:
            return False

    return pron_dict[word1] == pron_dict[word2] == pron_dict[word3]

def find_homophones_words(pron_dict):
    """Returns a list of words, where:

    * word
    * word with the first item omitted
    * word with the second item omitted

    are all homophones and existing in pron_dict.

    pron_dict: dict
    """

    homophones = []

    for word in pron_dict:
        first_word = word[1:]  # word with the first letter omitted
        second_word = word[0] + word[2:] # word with the second letter omitted
        if are_homophones(pron_dict, word, first_word, second_word):
                homophones.append(word)

    return homophones

pron_dict = read_dict('CMUDict.txt')
homophones = find_homophones_words(pron_dict)

for word in homophones:
    print([word, word[1:], word[0] + word[2:]])

Notes:

I haven't, strictly, followed the puzzler. The code finds words satisfying
the main requirements, except that the word is "one-syllable and five-letters".
I'm a hobbyist and a beginner.
I don't only want to optimize the code, but also my skills and style. Therefore any notes, advice and suggestions aren't only welcomed, but also encouraged!

There's an efficiency improvement you could make to are_homophones: As is, you check that all three words are in pron_dict before checking that they're the same. However, it's better practice to catch errors using try blocks rather than trying to prevent them from happening (see this Stack Overflow post). A cleaner/faster way way to write this would be

def are_homophones(pron_dict, word1, word2, word3):
    try:
        return pron_dict[word1] == pron_dict[word2] == pron_dict[word3]
    except KeyError:
        return False

It's also good practice to use with blocks to open files so that Python automatically handles closing the file. With this change, read_dict becomes

def read_dict(file_path):
    pron_dict = {}

    with open(file_path) as fin:
        for line in fin:
            index = line.find(' ')  # e.g. "AA  EY2 EY1"
            word = line[:index]
            pron = line[index+2:]
            pron_dict[word] = pron

    return pron_dict


Binary search with strings I wrote a quick and dirty program to perform binary search with an ordered array (list) of C style strings.  For example:

{"1", "12", "23", "124"} etc (though my input set is actually much different)

NOTE:

Apologies for the confusing names - string is not the string, string->ID is. (Like I said, I cobbled it together very quickly since I was writing into t a larger program). Assume all variables have been declared.
I only care about program (piece) correctness and efficiency of the algorithm (for example - ignore the multiple returns etc) at the moment.
Don't worry about function calls like Log() or macros that I've used.  Bottom and top are calculated off of stringEntries.
Think of ways this will break. Don't worry about the rest of the program - just focus on this piece!
pID is your input string. We're searching through "string"'s attribute "ID" (which is the string). Assume that "string" is actually something else, like a ptr to a struct or something, with a member "ID" which is a C-style string (null terminated and everything).
len is actually strlen(pID).

    while (TRUE)
            {

      *string = &stringEntries[mid = (bottom + top) / 2];

       if (*string == NULL) {
              Log(DEBUG, _ERROR,”*string is NULL\n”);
              return ERROR;
       }
       else if ((*string)->ID == NULL) {
              Log(DEBUG, _ERROR,”(*string)->ID is NULL\n”);
              return ERROR;
       }

       if (!(diff = strcmp(pID, (*string)->ID)))
               diff = len - strlen((*string)->ID);

       if (!diff) return PRESENT;

       if (bottom == top) {
              if (diff > 0)
                     (*string)++;  //ignore this bit - i need it later on
                     return NOT_PRESENT;
       }

       if (diff > 0){
              if (bottom == mid) bottom++;
              else  bottom = mid;
       }
       else        top = mid;
    }

There is not enough information to give a real review.
If you fill in all the type information and make it a compilable function I will review your algorithm, but based on the available code the only thing I can review is the style.

C and C++ are two completely different languages.

Pick one and learn it.
The syntax may be superficially the same but how you use it will not be.
I will make my comments assuming you are trying to write C++ not C.

Your layout is so horrible that it is practically unreadable.

Pick one of the common indenting style
If this is a problem with cut and paste then learn how to use your editor so that you can cut/paste without tabs onto a website.

Review

while (TRUE)

C++ has it's own boolean types. true/false

        {

Horrible formatting. The above '{' is indented way to far especially  in relation to the line it is encapsulating.

  *string = &stringEntries[mid = (bottom + top) / 2];

Assignment inside the [] is allowed but makes the code hard to read.
Prefer not to do that it is much more readable to split that into two lines.
You are not being paid to write compact code you are being paid to write easy to maintain code.

   if (!(diff = strcmp(pID, (*string)->ID)))
           diff = len - strlen((*string)->ID);

Your naming of variables here makes this code unreadable.
Make your variable names obvious.
Don't re-use variables. In the above code it looks like diff is being used for two different purposes (or is it not?). Let the compiler optimize out the extra space you should be making the code clear.

   if (bottom == top) {
          if (diff > 0)
                 (*string)++;  //ignore this bit - i need it later on
                 return NOT_PRESENT;

Absolute fail.
The indention suggests that we return when (diff > 0) but the code is not going to do that, as only the first statement after the if statement belongs to the if block. Always prefer to use statement blocks after control structures. Even if they are one liners (some people still use multi-line macro's that will break your code if you don't place it in '{' '}'

   if (diff > 0){
          if (bottom == mid) bottom++;
          else  bottom = mid;
   }
   else        top = mid;

More barely readable code.
A nice alignment would be good. There are also other constructs that will help:

  if (diff > 0)
  {
      bottom = ( bottom == mid) ? bottom + 1 : mid;
  }
  else
  {
      top    = mid;
  }


Quicksort implementation in JavaScript I implemented Quicksort in JavaScript. I'm trying to follow the best principles in JavaScript. In Ruby when you declare classes your able to use private  for the methods that are private. I don't think there is anything like that in JavaScript.
class QuickSort {
  sort(a) {
    a = this.shuffle(a); //Shuffling is needed for perfomance guarantee.
    this.privateSort(a,0, a.length - 1)
    return a;
  }

  privateSort(a, lo, hi) {
    if(hi <= lo) { return }
    let j = this.partition(a, lo, hi);
    this.privateSort(a, lo, j - 1)
    this.privateSort(a, j + 1, hi)
  }

  partition(a, lo, hi) {
    let i = lo;
    let j = hi + 1;
    let v = a[lo];

    while (true) {
      while (a[++i] < v) {
        if (i >= hi) { break; };
      }
      while (v < a[--j]) {
        if (j <= lo) { break; };
      }
      if (i >= j) { break; }
      this.swap(a, i, j);
    }
    this.swap(a, lo, j);
    return j;
  }

  swap(a, firstEl, secondEl) {
    let temp = a[firstEl];
    a[firstEl] = a[secondEl];
    a[secondEl] = temp;
    return
  }

  // Knuth-Fisher-Yates shuffle algorithm
  shuffle(arr) {
    for (var i = arr.length - 1; i > 0; i--) {
      const swapIndex = Math.floor(Math.random() * (i + 1))
      const currentEl = arr[i]
      const elToSwap  = arr[swapIndex]

      arr[i] = elToSwap;
      arr[swapIndex] = currentEl;
    }
    return arr;
  }
}

I'm also thinking of adding a method to verify input, make sure input is array, not nil, etc. Ideas for how I can go about implementing such method are welcome.
Is there anything I can improve, are there principles being violated? Am I following best practices in class implementation in JavaScript.
here is a test:
var obj = new QuickSort();
sample = obj.sort(sample);
console.log(sample)

I'm pretty sure the js convention is still to prefix "private" stuff with an underscore.
class QuickSort {
  sort(a) {...}
  _sort(a, lo, hi) {...}
  ...}

The only other thing that sticks out to me is your use of ++ and -- in the while loops. While perfectly idiomatic, I think it would express the logic better to move both conditionals together in the while(condition), and the increment action into the while's body. Of course it's always nice if one can abstract away a loop altogether...
Other people can speak more specifically about javascript than I can.


Localizing an enum in Java I am currently using an enum in my Android application to display time periods:

Weekly
Bi-Weekly
Monthly
Quarterly
Yearly

I created the enum like this, because I wanted to know the description, and (roughly) number of days:

public enum SpendingPeriods {
    WEEKLY("Weekly", 7),
    BIWEEKLY("Bi-Weekly", 14),
    MONTHLY("Monthly", 30),
    QUARTERLY("Quarterly", 120),
    YEARLY("Yearly", 365);

    private final String description;
    private final int numDays;

    SpendingPeriods(String description, int numDays){
        this.description = description;
        this.numDays = numDays;
    }

    public String getDescription(){
        return description;
    }

    public int getNumDays(){
        return numDays;
    }
}

Now, I would like to localize the enum, and so far my only solution has been to remove the description from the constructor, and check Locale and switch based on number of days:

SpendingPeriods(int numDays){
        this.numDays = numDays;
        if(Locale.getDefault().getLanguage().equals("fr")){
            switch(numDays){
                case 7:
                    this.description = "Hebdomadaire";
                    break;
                case 14:
                    this.description="Bimensuel";
                    break;
                case 30:
                    this.description="Mensuel";
                    break;
                // Others
            }
        } else{
            // Fall back to English for unsupported languages
            switch(numDays){
                case 7:
                    this.description = "Weekly";
                    break;
                case 14:
                    this.description = "Bi-Weekly";
                    break;
                case 30:
                    this.description = "Monthly";
                    break;
                // Others
            }
        }
    }

I was hoping there would be a way to do this using a string-array in an XML file, but I can't seem to find anything like that. Is this okay practice? Is there a better way?

Don't localize your Java code.  You don't want to hand over your source code to your translator, and you don't want to recompile the code to add a support for a new language.  Rather, you should internationalize your code and do all the localization through resources, as recommended in the Android Developers documentation.

Furthermore, localizing strings in isolation doesn't work in the general case.  For example, "monthly" may need to be translated as « mensuel », « mensuelle », « mensuels », « mensuelles », or « mensuellement », depending on context.  Word order also tends to differ between languages.  Therefore, the only sane approach is to translate entire phrases at the point of use.

res/values/strings.xml

<resources xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
    <string name="expenses">Your {0,choice, 1#weekly| 2#bi-weekly| 3#monthly| 4#quarterly| 5#annual} expenses are {1,number,currency}.</string>
</resources>

res/values-fr/strings.xml

<resources xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
    <string name="expenses">Vos dépenses {0,choice, 1#hebdomadaires| 2#bimensuelles| 3#mensuelles| 4#trimestrielles| 5#annuelles} sont {1,number,currency}.</string>
</resources>

Java code

import java.text.*;
…

Format expensesFmt = new MessageFormat(getResources().getString(R.string.expenses));
textView.setText(expensesFmt.format(new Object[] { MONTHLY.ordinal(), 1337 }));

… to get

Your monthly expenses are $1337.00.

or

Vos dépenses mensuelles sont 1337,00€.


Abstract factory pattern for a system modelisation I've just done an exercise on the abstract factory pattern and I want someone to tell me whether or not I followed the rules of the abstract factory pattern.

Exercise 2 (Abstract Factory: GUI based OS)
The purpose of this exercise is to provide a modelisation of a system for creating different components of a graphical user interface (buttons, menu,...) depending on the operating system (Linux, Windows, ...) to which they are intended.
The model must create components without having to know their concrete classes, to be independent of how the components evolve. The Abstract Factory pattern seems well suited.

Make the UML diagram of this program. We consider that the buttons have a color, width and height, and menus just wide.

Write the Java classes d'efinies by the UML diagram.

Add back a new operating system (Mac OS X, for example)

The abstract class Composant:
package Interfaces;

public abstract class Composant {
    protected double largeur ;
    protected String color = null;
    protected double hauteur ;

    public abstract void setColor(String C);
    public abstract void setLargeur(double L);
    public abstract void setHauteur(double H);

    public abstract String getColor();
    public abstract double getLargeur();
    public abstract double getHauteur();
}

The Boutton class:
package concreteClasses;

import Interfaces.Composant;

public class Boutton extends Composant{

    @Override
    public void setColor(String C) {
        color = C;
    }

    @Override
    public void setLargeur(double L) {
        largeur = L;
    }

    @Override
    public void setHauteur(double H) {
        hauteur = H;
    }

    @Override
    public String getColor() {
        return color;
    }

    @Override
    public double getLargeur() {
        // TODO Auto-generated method stub
        return largeur;
    }

    @Override
    public double getHauteur() {
        // TODO Auto-generated method stub
        return hauteur;
    }
    public String toString(){
        return "boutton color ="+color+" boutton largeur ="+largeur+" boutton     hauteur ="+hauteur;
    }
}

The Menu class:
package concreteClasses;

import Interfaces.Composant;

public class Menu extends Composant{

    @Override
    public void setColor(String C) {
        color = null;
    }

    @Override
    public void setLargeur(double L) {
        largeur = L;
    }

    @Override
    public void setHauteur(double H) {

    }

    @Override
    public String getColor() {
        return color;
    }

    @Override
    public double getLargeur() {
        // TODO Auto-generated method stub
        return largeur;
    }

    @Override
    public double getHauteur() {
        return hauteur;
    }
    public String toString(){
        return "Menu largeur ="+largeur;
    }
}

The abstract class AbstractFatcory:
package factoryClasses;

import Interfaces.Composant;

public abstract class AbstractFactory {
    public abstract Composant getComposant(String composant);
}

The FactoryComposant class:
package factoryClasses;

import concreteClasses.*;
import Interfaces.Composant;

public class FactoryComposant extends AbstractFactory{

    @Override
    public Composant getComposant(String composant) {
        if(composant.equalsIgnoreCase("Boutton")) return new Boutton();
        else if(composant.equalsIgnoreCase("Menu")) return new Menu();
        return null;
    }
}

The Main class:
import Interfaces.Composant;
import factoryClasses.*;

public class Main {
    public static void main (String[]args){

        AbstractFactory factorycomposant = new FactoryComposant();
        Composant compo = factorycomposant.getComposant("Boutton");
        compo.setColor("Red");
        compo.setHauteur(12);
        compo.setLargeur(12);
        System.out.println(compo);

        Composant compo2 = factorycomposant.getComposant("Menu");
        compo2.setLargeur(10);
        System.out.println(compo2);
    }
}

First of all: You got it nearly right.

One thing in your implementation deviates from the standard implementation:

@Override
public Composant getComposant(String composant) {
    if(composant.equalsIgnoreCase("Boutton")) return new Boutton();
    else if(composant.equalsIgnoreCase("Menu")) return new Menu();
    return null;
}

You parameterize your factory methods with a string. Don't do that in this case. Passing a parameter to a factory may be possible but the products should be "strong" related to each other. Everybody who wants to know what your factory can produce should see it by the method signatures provided by the abstract factory and not guess (or read in the documentation... urgh). If you want to produce two different items, create another factory method. For a factory "FactoryX" this should look like this:

@Override
public Button createBoutton() {
    return new BouttonX();
}

@Override
public Menu createMenu() {
    return new MenuX();
}

One other thing: the abstract Composant-class is irrelevant to the abstract factory in your case as far as I see. Furthermore it will bind you to a set of methods that "Menus" and "Bouttons" have in common. This would hinder you to configure special properties of "Bouttons" and "Menus". The two types may of course stay derived from Composant.

I thought about it a little bit when the use of parameters in the factory pattern may be appropriate. In the following example (in pseudo code) a LabelField or a LabelArea is returned which depends on the size of the given text.

@Override
public AbstractLabel getAbstractLabel(String text) {
    if (text.length > 120) {
        return new LabelArea(text); // multi line presentation
    } else {
        return new LabelField(text) // single line presentation
    }
}

The parameter is not technically connected to the return value but semantically. You can see the strong relationship between those return values. They were built to present text.


Euclidean algorithm to determine GCD (greatest common divisor) in C -- alternative method Been looking into some simple algorithms, I came up with the following piece of code in C to implement the Euclidean algorithm iteratively (I like to be clear here, I don't want the recursive implementation for now).
I've seen various other implementations and in fact most books prefer a different pseudo code.
I'm wondering if the following implementation is any better or worse?
#include <stdlib.h>

void swap_numbers(void *a, void *b)
{
  int temp = *(int *)a;
  *(int *)a = *(int *)b;
  *(int *)b = temp;
}

unsigned euklid_iterative(unsigned a, unsigned b)
{
  while ((a = a % b))
    swap_numbers(&a, &b);
  return b;
}

Unnecessary function calls
My quick thought is that you are making an unnecessary function call.
unsigned temp;
while ((temp = a % b)) {
    a = b;
    b = temp;
}

The thing with the doubled parentheses is a hack to keep some compilers from warning you that you may be doing an unintentional assignment.  It should make no functional difference.
This does fewer assignments.  Three compared to four in your version.  And it makes no function calls.  Typically a function call is more involved than an assignment.  Because it has to first save the current state, then jump somewhere else and do work, and finally restore the original state.  The save and restore steps themselves often involve assignments.  And those may be relatively expensive assignments from register to cache or memory.  Whereas that loop can be done with three registers (or even two if the compiler optimizes out temp by doubling the code).
while ((a = a % b)) {
    if (!(b = b % a)) {
        return a;
    }
}

No temp in that version, but duplicate logic.  I wouldn't normally recommend writing that code.  But it would be a perfectly legitimate compiler optimization if there is a shortage of available registers.
Note:  if you only call this function a few times, this won't make a difference.  Use whatever you find more readable.  But if you are calling GCD in your main loop, it is possible that efficiency will matter.  If efficiency does matter, my revised version (the first one) will (absent compiler optimizations) probably be faster than your version.  While premature optimizations are to be avoided, there is little harm in picking the more efficient version if you have two working versions.  And GCD is exactly the kind of problem that may be subject to profiling that would lead to that kind of optimization.  So many sources may offer the best optimized algorithm rather than an alternative.
I find it a best practice to never use the statement form of control structures and always use the block form.  I also prefer the brackets on the same line as the control structure.  The latter is very much opinion, but the former is based on actual experience with bugs caused by editing.
Be careful of types
I also find it a bit risky that your variables are unsigned in the original function but int in the swap_numbers function.  This presumably works because the unsigned and int types use the same storage width.  But that's an implementation detail.  Is that implementation detail guaranteed always?  I don't know off-hand.  Perhaps you already did that work and do know.  But you don't say that you know and there's no comment explaining when you can and cannot use swap_numbers.
For example, what would happen if used with an unsigned long?  On some systems, that might work because int and long are the same width.  On other systems, you may only swap part of the numbers.  Possibly only some of the time (because if the bytes that you are swapping happen to be the ones with data, it might not matter).
You might fix this by switching from pointers to handles (pointers to pointers).
void swap_numbers(void **a, void **b)
{
  void *temp = *a;
  *a = *b;
  *b = temp;
}

unsigned euklid_iterative(unsigned *a, unsigned *b)
{
  while ((*a = *a % *b))
  {
    swap_numbers(&a, &b);
  }

  return *b;
}

But it might be easier just to write a swap_unsigned function.  Or write out the swap entirely as I did originally.
All code in this post is untested.  Not even for compilable syntax much less correct logic.  Use at your own risk.


Many DB queries and conditions I don't know it is some design pattern for it or I can't do it in a different way the code looks like this:
Iteam iteam = itemRepository.findByNameAndNoAndCountryCodeAndCurrencyCode(name, countryCode, currencyCode);
    if(item == null){
        item = itemRepository.findByNameAndNoAndCountryCodeAndCurrencyCode(name, "US", currrencyCode)
    }
    if(item == null){
        item = itemRepository.findByNameAndNoAndCountryCodeAndCurrencyCode(name, countryCode, "USD")
    }
    if(item == null){
        item = itemRepository.findByNameAndCountryCode(name, countryCode)
    }
    if(item == null){
        item = itemRepository.findByNameAndCurrencyCode(name, currrencyCode)
    }

And I have a lot of this (like 15) kind of queries with the condition it is possible to do not make to many conditions and use some trick for this code?

As suggested by @911992, why not moving that "logic" into the database. Not as a function/stored-procedure/view but as a combination of all your queries :
SELECT * FROM items WHERE name=? AND country_code=? AND currencyCode=?
UNION
SELECT * FROM items WHERE name=? AND country_code='US' AND currencyCode=?
UNION
SELECT * FROM items WHERE name=? AND country_code=? AND currencyCode='USD'
UNION
SELECT * FROM items WHERE name=? AND country_code=?
UNION
SELECT * FROM items WHERE name=? AND currencyCode=?

You can limit that query to one row to avoid too much results.
If want to keep everything into the code and are looking for a design pattern, you can search for the chain of responsibility.


jQuery auto increment for class name I'm trying to make a drop down menu with click event. I have this HTML structure:

            Menu 1

Submenu 1

Submenu 1

            Menu 2

Submenu 2

            Menu 3

Submenu 3

Submenu 3

I want to avoid the double drop down, so I made this script:

$(document).ready(function(){
            $('li.submenu1').hide();
            $('li.menu1').click(function(e){
                $(this).find('li.submenu1', this).slideToggle('fast');
                $('li.submenu2').hide();
                $('li.submenu3').hide();
                e.stopPropagation();
            })
            $('li.submenu2').hide();
            $('li.menu2').click(function(e){
                $(this).find('li.submenu2', this).slideToggle('fast');
                $('li.submenu1').hide();
                $('li.submenu3').hide();
                e.stopPropagation();
            })
            $('li.submenu3').hide();
            $('li.menu3').click(function(e){
                $(this).find('li.submenu3', this).slideToggle('fast');
                $('li.submenu1').hide();
                $('li.submenu2').hide();
                e.stopPropagation();
            })
        })

Is there any way to simplify the code? Perhaps some auto-increment tricks will help.

Probably something like this would enable you to extract it. Basically I'd suggest looping through hiding everything then setting the one you want to open, you could be clever and check if the menu selected is already one, but hopefully this will give you the idea:

$("menubar ul").each(function(){
   $("li", this).click({
      $("menubar ul").each(function(){
         $("li ul", this).hide()
      })
      $("ul",this).toggle(...
   })
})


Case-insensitive String search in Array Wrote this function for a Stack Overflow answer. It started off pretty bad but in the end I polished it a bit and now I have something I think to be pretty clean. Actually, when I look at the code, this is what I usually write when I'm programming AS3 (minus the documentation, I'm a lazy bastard, although that's slowly changing too).

The problem the code attempts to solve is finding a case-insensitive string in an array.

Soo... anything that can be improved here?

Should I handle null and undefined?

Does my documentation look helpful from a IDE usage perspective?

/**
 * Searches through an array for a case-insensitive string match.
 * Attempts to imitate Array.indexOf where it can.
 * @param arr The array to search through
 * @param searchingFor The string to search for
 * @param fromIndex Optional, an index to start searching at.
 * @returns The index of the array that a match was found at (zero-indexed), or -1 if no match was found.
*/
public static function indexOfCaseInsensitiveString(arr:Array, searchingFor:String, fromIndex:uint = 0):int {
    var lowercaseSearchString:String = searchingFor.toLowerCase();
    var arrayLength:uint = arr.length;//Retrieving array length is expensive, optimization

    for(var index:uint = fromIndex; index < arrayLength; index++) {
        var element:* = arr[index];
        if(element is String && element.toLowerCase() == lowercaseSearchString) {
            return index;
        }
    }
    return -1;//It wasn't found in the array.
}

Documentation
/**
 * Searches through an array for a case-insensitive string match.

Excellent!

 * Attempts to imitate Array.indexOf where it can.

I'm not quite sure what you want to say with this.

 * @param arr The array to search through

There's no need to shorten the variable name a few characters. array is a better name, but still says absolutely nothing about what it is used for. In this case, I actually have to say that I like the variable names that PHP uses. haystack. Can it be any clearer? Nearly everyone knows about the saying "Finding a needle in a haystack". So the name haystack is a quite good one.
 * @param searchingFor The string to search for

Using the needle-haystack analogy, this can be called needle.

 * @param fromIndex Optional, an index to start searching at.

From an IDE perspective, I think it is not necessary to state that the parameter is optional. All ActionScript IDEs that I know of have built-in features for this, by stating parameter = defaultValue. Your documentation doesn't state what the default value is, so the IDE is more helpful here than your documentation. Of course though, one would expect it to start at index 0. I would follow how others have done and simply state "index to start searching at." Or, even better "The location in the array from which to start searching for the item."

 * @returns The index of the array that a match was found at (zero-indexed), or -1 if no match was found.

First of all, everyone who uses arrays should be aware that array indexes in AS3 is zero-indexed. Secondly, the term 'index' was also mentioned in the previous documentation line but there was no stating there that it was zero-indexed. I would leave the (zero-indexed) part out. The rest is perfectly fine, and is what one would expect out of the function.
Code
I would prefer a space after each colon, but that's just my personal preference. At least you are consistent about it.
It's great that you use uint in your code and not int or some other, shorter, type. A minor problem would be if the match was found at index >= 2147483648 though, as you return an int. This is however the same in many other indexOf functions because you want to return -1 if no match was found, and I doubt that you will get an array of length > 2147483647 as input. Also, retuning Number would be horribly overkill so there is really no other option here.
I personally find your comments in the code unnecessary, but they are definitely not "clutter" so they are fine. Although //It wasn't found in the array. can be removed as that behavior is described in the documentation of your method.
Now, there's only one thing left to say; Why not use the Vector.<String> type? Then you wouldn't have to check element is String for each element. Also, IIRC, Vector is faster than Array.
Overall, well done. Your code looks quite fine overall.
Null and undefined
You explicitly ask this question:

Should I handle null and undefined?

I would recommend that you at the beginning of the method check if the array or the search string is null or undefined and that you throw an error with a clear message if that is the case. It is often frustrating when a method throws an error because something is null, but as it is automatically thrown, it is harder for the programmer to find out what is null. Checking for preconditions at the start of the method is good practice!


Function for calculating Archimedean density functions I am not sure whether this is the right place to ask this, so please correct me if it's not.

I am following this paper (e.g., Corollary 3.3) for some derived functional representations of the densities and writing code for those. The code for one of the families:

(x will be a vector of 1xd between [0,1], tau - scalar in [1, infty).)

'joe_density' <- function(x, tau){
  require(gmp)
  require(Rmpfr)
  require(magrittr)

  d <- length(x)

  P_d <- function(x, tau, d){
    alpha <- 1/tau
    lapply(1:d, function(k){
      as.numeric(gmp::Stirling2(n = d, k = k)) * gamma(k - alpha) / gamma(1 - alpha) * x^(k-1)
    }) %>% do.call(sum, .)
  }

  h <- prod((1 - (1 - x)^tau))

  mpfr( tau^(d-1), 16) *
    mpfr( prod((1 - x)^(tau - 1)), 16) /
    mpfr( (1 - h)^(1 - 1/tau), 16) *
    mpfr( P_d( h/(1-h), tau, d), 16)
}

Example call:

# Simple case
x <- pnorm(rnorm(100))
joe_density(x, tau = 2)

# Longer performance time when looping, d=2
expand.grid(x = seq(-5,5,by=0.1), y = seq(-5,5,by=0.1)) %>%
  apply(.,1,function(z){
     x <- pnorm(z)
     joe_density(x, tau = 2)
  })

In essence, the typical calculations require tons of looping, gamma/Stirling numbers, and I have noticed that the default precision is usually not enough, hence the Rmpfr usage. One time evaluation of the code is not a problem, however, the calculations become very lengthy when the function is called thousands of time (e.g., passing it to  optim and the likes).

Are there simpler ways to program this? This particular function is just an example, but most of the different functions follow similar approach, with a high usage of Stirling/gamma, lengthy products and etc.

Could, for example, rewriting parts of the code in Rcpp be helpful, or is this not the type of task that we could expect  gains here? I have no experience in writing cpp code, so am not sure what to expect.

Some suggestions:

split code more, line by line, so it is easier to profile and see bottlenecks
use profvis
use %>% less often if that part of code is called frequently, it has some overhead
separate function definitions

This should run a little bit faster: (15s vs 27s on your example)

P_d <- function(x, tau, d){
  alpha <- 1/tau
  l <- sapply(1:d, function(k){
    p1 <- Stirling2(n = d, k = k)
    p1 <- as.numeric(p1)
    p1 * gamma(k - alpha) / gamma(1 - alpha) * x^(k-1)
  })
  sum(l)
}

joe_density <- function(x, tau){
  d <- length(x)
  h <- prod((1 - (1 - x)^tau))

  v1 <- tau^(d-1)
  v2 <- prod((1 - x)^(tau - 1))
  v3 <- (1 - h)^(1 - 1/tau)
  v4 <- P_d( h/(1-h), tau, d)

  p1 <- mpfr(v1, 16) # slow
  p1 * v2 / v3 * v4 # slow
}

The slowest part is mpfr & the last line, because each value is converted to mpfr.
Maybe you can deal with precision loss in some kind of different way?


Proper use of tail recursion while forming a list As a project to better develop my understanding of functional programming, I am writing a prime number generator in Scheme. I am using a simple brute-force algorithm to detect whether a number is prime and I know there are better algorithms. What I'm interested is whether my attempt at the brute-force algorithm can be better written and whether I am using tail recursion appropriately and forming the list efficiently. The goal is to have a list of numbers at the end I can use for some task or print out. Maybe even write it as a generator where I can continually ask it for the next prime. Here is my current code:

(define is_prime1
 (lambda(num div)
  (if (= num div)
   #t
   (if (= (remainder num div) 0)
    #f
    (is_prime1 num (+ div 1))
   )
  )
 )
)

(define is_prime
 (lambda(num)
  (if (< num 2) #f (is_prime1 num 2))
 )
)

(define list_primes1
 (lambda(idx num)
  (if (<= idx num)
   (if (not (is_prime idx))
    (list_primes1 (+ idx 1) num)
    (cons idx
     (list_primes1 (+ idx 1) num)
    )
   )
   '()
  )
 )
)

(define list_primes
 (lambda(num)
  (list_primes1 0 num)
 )
)

(define print_primes
 (lambda(primes)
  (if (null? primes)
   '()
   (list
    (display (car primes))
    (newline)
    (print_primes (cdr primes))
   )
  )
 )
)

(print_primes (list_primes 4095))

Your algorithm is okay, but the real issue with this code is that your code style is atrocious, bordering on unreadable. Your style exhibits four main problems:

Scheme is not C, and parentheses are not curly braces. Do not put close parens on separate lines. Scheme code is intended to be read primarily by observing indentation, which brings me to the next point.
Your indentation is wrong. Again, Scheme is not C, and expressions should be indented to align subexpressions, not with a consistent 1-, 2- or 4-space indent. For an explanation of why this is so important, see this Stack Overflow answer.
This is a less significant point, but your explicit use of lambda here is unnecessary. There is a shorthand form with define that is equivalent to define paired with lambda, and it’s more idiomatic and easier to visually parse.
Identifiers in Scheme should be hyphenated (lisp-case), not separated by underscores (snake_case).

Just following the above formatting changes, your code becomes much more readable to a Schemer:

(define (is-prime1 num div)
  (if (= num div)
      #t
      (if (= (remainder num div) 0)
          #f
          (is-prime1 num (+ div 1)))))

(define (is-prime num)
  (if (< num 2) #f (is-prime1 num 2)))

(define (list-primes1 idx num)
  (if (<= idx num)
      (if (not (is-prime idx))
          (list-primes1 (+ idx 1) num)
          (cons idx
                (list-primes1 (+ idx 1) num)))
      '()))

(define (list-primes num)
  (list-primes1 0 num))

(define (print-primes primes)
  (if (null? primes)
      '()
      (list (display (car primes))
            (newline)
            (print-primes (cdr primes)))))

(print-primes (list-primes 4095))

Now we can focus on some more substantive code improvements. First of all, print-primes is an easy candidate for elimination. Not only does it needlessly produce a list, it can be trivially implemented using the for-each higher-order function. The name is also silly, since it does not print primes, it prints each element of a list. There is no reason to include the word “primes” in the name.

Instead, just replace the whole function with a simple use of for-each:

(define (displayln x) (display x) (newline))
(for-each displayln (list-primes 4095))

Next, let’s look at the bulk of the code. You have defined list-primes in terms of a helper functions, list-primes1. Since all your list-primes function is doing is calling list-primes1 with some arguments set, you can replace the helper function with a use of “named let”:

(define (list-primes num)
  (let loop ((idx 0)
             (num num))
    (if (<= idx num)
        (if (not (is-prime idx))
            (loop (+ idx 1) num)
            (cons idx
                  (loop (+ idx 1) num)))
        '())))

You also have a pair of nested ifs, which might be more readably  represented with a cond:

(define (list-primes num)
  (let loop ((idx 0)
             (num num))
    (cond
      ((> idx num)
       '())
      ((is-prime idx)
       (cons idx (loop (+ idx 1) num)))
      (else
       (loop (+ idx 1) num)))))

Taking a look at is-prime and is-prime1, we can once again replace is-prime1 with a use of named let:

(define (is-prime num)
  (if (< num 2)
      #f
      (let loop ((num num)
                 (div 2))
        (if (= num div)
            #t
            (if (= (remainder num div) 0)
                #f
                (loop num (+ div 1)))))))

However, this is still much more complicated than it needs to be. Due to how Scheme’s and and or are both short-circuiting and implement “truthiness”, where all non-#f values are truthy, we can replace most of the uses of if with and or or:

(define (is-prime num)
  (and (>= num 2)
       (let loop ((num num)
                  (div 2))
         (or (= num div)
             (and (not (= (remainder num div) 0))
                  (loop num (+ div 1)))))))

Also, we can replace (= x 0) with the zero? predicate to improve readability:

(define (is-prime num)
  (and (>= num 2)
       (let loop ((num num)
                  (div 2))
         (or (= num div)
             (and (not (zero? (remainder num div)))
                  (loop num (+ div 1)))))))

Now, it’s worth noting that list-primes is not tail recursive, since in the second cond case, a call to cons is in tail position, not loop. A way to make this function tail recursive is to build up a list iteratively, then reverse it at the end:

(define (list-primes num)
  (reverse
   (let loop ((idx 0)
              (num num)
              (acc '()))
     (cond
       ((> idx num)
        acc)
       ((is-prime idx)
        (loop (+ idx 1) num (cons idx acc)))
       (else
        (loop (+ idx 1) num acc))))))

This leaves us with the final, completely tail-recursive program:

(define (is-prime num)
  (and (>= num 2)
       (let loop ((num num)
                  (div 2))
         (or (= num div)
             (and (not (zero? (remainder num div)))
                  (loop num (+ div 1)))))))

(define (list-primes num)
  (reverse
   (let loop ((idx 0)
              (num num)
              (acc '()))
     (cond
       ((> idx num)
        acc)
       ((is-prime idx)
        (loop (+ idx 1) num (cons idx acc)))
       (else
        (loop (+ idx 1) num acc))))))

(define (displayln x) (display x) (newline))
(for-each displayln (list-primes 4095))


Get an integer as input and display Pi rounded to that amount of decimal places I've been writing a program that accepts an integer as input and displays the number Pi rounded to that number. The only issue I see is the fact that the Math.Round method can only round up to 15 spaces and there's no try-catch for that ArgumentOutOfRange exception. I'm also not sure how safe it is letting your flow of execution rely on a try-catch statement.

class Program
{
    public static int counter = 0;
    static void Main(string[] args)
    {
        Console.WriteLine("Welcome to the Pi-Rounder! Find Pi to up to 15 decimal places!");
        Console.WriteLine("Please enter the number of decimal places you'd like to round to.");
        int roundTo;
        do
        {
            string digitAsString = Console.ReadLine();
            roundTo = ConversionLoop(digitAsString);
        }
        while(roundTo == 0 && counter != 5);
        if(counter == 5)
        {
            throw new FormatException();
        }
        else
        {
            double piRounded = Math.Round(Math.PI, roundTo);
            Console.WriteLine(piRounded);
            Console.ReadLine();
        }
    }

    static int ConversionLoop(string digitString)
    {
        try
        {
            int digit = Convert.ToInt32(digitString);
            return digit;
        }
        catch(FormatException)
        {
            counter++;
            Console.WriteLine("That was not a valid number. Please try again.");
            return 0;
        }
    }
}

There are several issues with this piece of code:

    do
    {
        string digitAsString = Console.ReadLine();
        roundTo = ConversionLoop(digitAsString);
    }
    while(roundTo == 0 && counter != 5);
    if(counter == 5)
    {
        throw new FormatException();
    }
    else
    {
        double piRounded = Math.Round(Math.PI, roundTo);
        Console.WriteLine(piRounded);
        Console.ReadLine();
    }

Problems:

"ConversionLoop" is a meaningless name. The function parses a string to an integer, so a better name would be toInt
The handling of invalid input and incrementing the counter are not visible here. At first look I didn't see how the counter can advance, and it seemed you don't tell the user about invalid results. I had to look at the ConversionLoop to find out, but it was not logical to do so. The responsibility of getting valid input should not be split between two methods, it would be clearer to handle in one place, and have all the elements of the logic easily visible.
If the user fails to enter valid input 5 times, the code throws new FormatException()

FormatException is not appropriate for this. The problem is not invalid format, but failure to enter valid input within a reasonable number of retries. It's a different kind of error, and should be captured by a different exception class
Creating an exception without a text message explaining the problem makes debugging difficult

After throwing an exception in the if branch, the program exits from the method, so you can simplify the else

A bug

I think you have a bug: if the user enters 0 as input,
the ConversionLoop method doesn't print an error and returns 0 normally,
but the program will still wait for another try.
Without a message, this will be confusing to the user.
I doubt you intended it this way.

Suggested implementation

With the above suggestions, the code becomes:

class UserInputException : Exception
{
    public UserInputException(string message) : base(message)
    {
    }
}

public static int MAX_TRIES = 5;

static void Main(string[] args)
{
    Console.WriteLine("Welcome to the Pi-Rounder! Find Pi to up to 15 decimal places!");

    int roundTo = ReadIntegerInput();
    double piRounded = Math.Round(Math.PI, roundTo);
    Console.WriteLine(piRounded);
    Console.ReadLine();
}

static int ReadIntegerInput()
{
    Console.WriteLine("Please enter the number of decimal places you'd like to round to.");
    int counter = 0;
    while (true)
    {
        string digitAsString = Console.ReadLine();
        try
        {
            return Convert.ToInt32(digitAsString);
        }
        catch(FormatException)
        {
            if (++counter == MAX_TRIES)
            {
                throw new UserInputException("Too many invalid inputs. Goodbye.");
            }

            Console.WriteLine("That was not a valid number. Please try again.");
        }
    }
}


ActiveRecord selection, modified by optional parameters I have an action method in my Rails controller which filters an ActiveRecord model depending on the submitted GET parameters.
Rubocop complains that Assignment Branch Condition Size is too high: 20.62/15.

Is there a way to improve the following code?

@measurements = Measurement.all
@measurements = @measurements.where(sensor_id: params[:sensor_id].split(',')) if params[:sensor_id].present?
@measurements = @measurements.last_per_sensor(params[:last_per_sensor]) if params[:last_per_sensor].present?
@measurements = @measurements.last(params[:last]) if params[:last].present?

Some notes:

That's too much code for a controller method, it should be moved to the model.
The same name (@measurements) holds up to 4 different values. As a general rule, avoid variable rebinding: different values, different names.
More about how the filtering method could be written keeping it DRY: https://stackoverflow.com/a/4480285/188031

For example:

class Measurement < ActiveRecord::Base
  def self.filter(attributes)
    attributes.select { |k, v| v.present? }.reduce(all) do |scope, (key, value)|
      case key.to_sym
      when :sensor_id
        scope.where(key => value)
      when :last, :last_per_sensor
        scope.send(key, value)
      else
        scope
      end
    end
  end
end

And in the controller simply:

@measurements = Measurement.filter(params.slice(:sensor_id, :last, :last_per_sensor))


Most elegant way to copy type alias class to original collection Looking for a most elegant way to copy dictionary type alias to original collection.
using System;
using System.Collections.Generic;
public class SmallDictionary: Dictionary<string, JToken>
public class NestedDictionary : Dictionary<string, SmallDictionaly>

using System.Collections.Generic;
using System.Linq;
using Microsoft.Azure.Adp.Configuration.Events.Entities;
using Newtonsoft.Json.Linq;

public static Dictionary<string, Dictionary<string, JToken>> ToDictionary(this NestedDictionary configurationEntry)
{
   Dictionary<string, Dictionary<string, JToken>> resultDictionary = new Dictionary<string, Dictionary<string, JToken>>();
   foreach (KeyValuePair<string, SmallDictionary> pair in configurationEntry)
   {
      resultDictionary.Add(pair.Key, pair.Value);
   }
   return resultDictionary;
}
```

You definitely don't want to extend a type in order to alias it.
This introduces complexity, is not idiomatic, and most importantly doesn't create an alias.
I believe what you're looking for is the type alias syntax
using System;
using System.Collections.Generic;

using SmallDictionary = Dictionary<string, JToken>;
using NestedDictionary = Dictionary<string, SmallDictionaly>;

As their name implies, type aliases do not create any new types, they simply introducing a scoped alias that refers to the aliased type.
The only real drawback to type aliases is that they cannot be generic.
Moving on the the logic itself,
There's no need to write a loop over the dictionary entries to copy them into it new dictionary.
Dictionary<TKey, TValue> provides a constructor which takes an IEnumerable<KeyValuePair<TKey, TValue> allowing us to specify its initial contents.
return new Dictionary<string, Dictionary<string, JToken>>(configurationEntry);

or
return new NestedDictionary(configurationEntry);

Thanks to the type alias


Is there a better way to handle multiple join statements in a linq query? I am rewriting a VB.NET app into C#. I won't subject you to the original code. I am mainly looking for a better way to handle all the join statements that are being done. I am dealing with a legacy database that I cannot currently change the structure of. (Though I am making my arguments for it)

I am wondering if there is a better / cleaner way to handle multiple join statements. I am using DB first EF 6.1 so I have more options than was originally available. Here is the original Linq query converted to C#:

(from myObject in _context.MyObjects
join myObjectType in _context.MyObjectTypes on myObject.CRTKey equals myObjectType.CRTKey
join myObjectsSchedule in _context.MyObjectSchedule on myObject.CRS_Key equals myObjectsSchedule.CRS_Key
join myObjectGroup in _context.MyObjectGroups on myObject.ReportKey equals myObjectGroup.ReportKey
where myObjectGroup.ReportGroupNameKey == groupNameKey
select myObject).Distinct();

This particular example only has three joins I have a few blocks of code that have in  excess of 6 join statements. Is there a better way to handle all the join statements?

I'm not sure what you mean by a 'cleaner way'. The syntax you're using is known as query syntax and is by far the best for readability when you have multiple joins.

If you use method syntax to express the same query you'll quickly see what I mean.

However, if you use method syntax you'll be able to break those joins up using different methods.

For example:

IGrouping<Results> AddJoin(IQueryable<Stuff> query) {

    if (business_rule) {
        return query.Join(inner => inner.Key, outer => inner.Key == outer.Key);
    }

    return query;
}

In this way you can reuse parts of your join and construct them dynamically.

Hope this helps.


Order list of movies and save sorting in cookies I did this project and I'm going back over it and trying to optimize some stuff. This is the movies_controller.rb file, which is basically a table that I need to sort based on user parameters. What I really want to know here, is there a better way to save and assign the @sort and @ratings_filter? I just need to save them between sessions.

This is my first "revised" attempt:

def index
  @movies = Movie.all
  @all_ratings = Movie.all_ratings
  @ratings_filter = @all_ratings

  (params[:sort]) ? (@sort = params[:sort]) : (@sort = session[:sort])

  (params[:ratings] and params[:commit] == 'Refresh') ?
  (@ratings_filter = params[:ratings].keys) : (@ratings_filter= session[:ratings] || @all_ratings)

  #save cookies
  if params[:sort] != session[:sort] or params[:ratings] != session[:ratings]
    session[:ratings] = @ratings_filter
    session[:sort] = @sort
    redirect_to movies_path :sort => @sort, :ratings => @ratings_filter# and return
  end

  @movies = Movie.sort_by_and_rating(@sort, @ratings_filter)

end

Here's the original for comparison:

    #params[:commit] has the value of the last button pressed
if (params[:ratings] and params[:commit] == 'Refresh')
    @ratings_filter = params[:ratings].keys
elsif params[:ratings]
    @ratings_filter = params[:ratings]
else
    @ratings_filter = session[:ratings] || @all_ratings
end

#@ratings_filter = params[:ratings].keys || session[:ratings] || @all_ratings

@sort = params[:sort] || session[:sort]

#save cookies
if params[:sort] != session[:sort] or params[:ratings] != session[:ratings]
    session[:ratings] = @ratings_filter
    session[:sort] = @sort
    redirect_to movies_path :sort => @sort, :ratings => @ratings_filter# and return
end

@movies = Movie.sort_by_and_rating(@sort, @ratings_filter)

Here's my GitHub if you need other files for context.

You could improve your code by doing the following:

Do not use and/or but &&/|| since what you need here is boolean operators, not flow control operators. Take a look at this article for a great example on how and when to use each operator.
You are constantly doing instance variable assignment at the one-liners conditionals which makes the code difficult to read (conditional ? @i = x ? @i = y). It's not wrong, but you could assign the result of the operation directly to the instance variable.
You could extract some conditional checks to private methods to state in a more clear way what your code does.
I'm not sure for this one, but I'll go for it. I think that you need the redirect when you set the session. You could just pass it to Movie.sort_by_and_rating(...).

Here's a refactored version of your code:

class MoviesController < ApplicationController
  def index
    @movies = Movie.all
    @all_ratings = Movie.all_ratings
    @sort = params[:sort] || session[:sort]
    @ratings_filter = params_rating? ? params[:ratings].keys : (session[:ratings] || @all_ratings)

    if set_cookies?
      session[:ratings] = @ratings_filter
      session[:sort] = @sort
    end

    @movies = Movie.sort_by_and_rating(@sort, @ratings_filter)
  end

  private

  def params_rating?
    params[:ratings] && params[:commit] == 'Refresh'
  end

  def set_cookies?
    params[:sort] != session[:sort] || params[:ratings] != session[:ratings]
  end
end


Formatting a phone number I have a program that will format a phone number in this format: (###)###-####

def obtain_phone_number
  loop do
   print 'Enter phone number: '
    phone_num = gets.chomp
    if !(phone_num[/\d{10}/])
      puts 'Invalid phone number, must contain 10 digits only'
    else
      formatted_phone_num = phone_num.insert(0, '(').insert(4, ')').insert(8, '-')
      return formatted_phone_num
    end
  end
end

When run:

irb(main):013:0> obtain_phone_number
Enter phone number: 1234567894
=> "(123)456-7894"
irb(main):014:0> obtain_phone_number
Enter phone number: 7894561234
=> "(789)456-1234"
irb(main):015:0> obtain_phone_number
Enter phone number: 5555555555
=> "(555)555-5555"

This works and all, and is great, but are there better ways to format a phone number? Something that's not as ugly as phone_num.insert(0, '(').insert(4, ')').insert(8, '-'). This just seems like it could be done in a more simplistic and easier way, and not to mention it will allow more than 10 digits.

You're already using a regex to check that the phone number is 10 digits, but you could also make it more precise, and use it to split the string apart:

def obtain_phone_number
  loop do
    print "Enter phone number: "
    phone_num = gets.chomp
    if phone_num =~ /^(\d{3})(\d{3})(\d{4})$/
      return "(#{$1})#{$2}-#{$3}"
    else
      puts "Invalid phone number, must contain 10 digits only"
    end
  end
end

When the =~ matching runs, it assigns the capture groups to the global $1, $2 etc. variables, so it's easy to glue a string back together from those parts. You can fiddle with it to make it accept more than 10 digits, if you want.

However, I would move the loop somewhere else, and just have the method return a formatted string or nil. Or possibly string, nil or raise an exception. If you get nil back, the user entered nothing (e.g. user cancelled/left it blank). If it raises an error, the user did input something, but it was invalid, and you can warn and retry.

Right now, if you call the method, you can never escape it without entering a number. I don't know the usage context for this, but I'd imagine it'd make sense to allow the user to skip somehow, rather than forcing them to input nonsense just to proceed.

So something like this might be better:

def obtain_phone_number
  phone_num = gets.chomp
  return nil if phone_num.empty?
  if phone_num =~ /^(\d{3})(\d{3})(\d{4})$/
    "(#{$1})#{$2}-#{$3}"
  else
    raise "Invalid input"
  end
end

# simple usage example
begin
  print "Enter phone number: "
  phone = obtain_phone_number
  puts phone
rescue StandardError
  puts "Invalid phone number, must contain 10 digits only"
  retry
end


All possible ways of merging two lists while keeping their order I am writing a function gives all possible ways of merging two ordered lists such that in the merged list, the elements are still ordered according to the elements in their respective starting list.

For example, merging (a b) with (1 2) would result in:

((a b 1 2) (a 1 b 2)(a 1 2 b) (1 a b 2) (1 a 2 b) (1 2 a b))

My initial intuition was to map over an indexing list:

(define (ordered-merge l1 l2)
  (if (null? l2)
      (list l1)
      (let ((num-list (enumerate 0 (length l1) inc identity)))
        (flatten (lambda (pos)
                   (map (lambda (result) (append (take l1 pos)
                                                 (cons (car l2) result)))
                        (ordered-merge (drop l1 pos) (cdr l2))))
                 num-list))))

enumerate is just a list-builder, which here will return a list (0 1 ... (length l1)).

But I'm having feelings this is probably trying to force imperative style into functional style.

Here's my second attempt:

(define (ordered-merge2 l1 l2)
  (cond ((null? l1) (list l2))
        ((null? l2) (list l1))
        (else
         (let ((insert-here (ordered-merge2 l1 (cdr l2)))
               (not-here (ordered-merge2 (cdr l1) l2)))
           (append (map (lambda (result) (cons (car l2) result))
                        insert-here)
                   (map (lambda (result) (cons (car l1) result))
                        not-here))))))

How could this be improved?

Since lists are built last to first and you want output to read first to last, we need to reverse the inputs. We will also need two accumulator (one for the permutation and one for results so far and some sort of recursion. Given that it's just a matter of messaging the logic into place.

(define (ordered-merge3 l1 l2)
  (let loop ((lefts (reverse l1)) (rights (reverse l2))
             (permu '()) (acc '()))
     (cond ((or (null? lefts) (null? rights))
            (let ((new-permu (if (null? lefts)
                                 (append (reverse rights) permu)
                                 (append (reverse lefts)  permu))))
               (cons new-permu acc)))
        ;;base case, if either lefts or rights is null there is only one in-order permutation that can be formed.
          (else (loop lefts
                      (cdr rights)
                      (cons (car rights) permu)
                      (loop (cdr lefts)
                            rights
                            (cons (car lefts) permu)
                            acc))))))

The last bit is the hard part to explain. When asked for the in-order permutation of lefts and rights, you can start forming the next permutation with either the car of lefts or the car of rights. In the nested loops here the inner loop is evaluated first because the scheme interpreter does eager evaluation. The value returned by examining the permutations that involve picking the left side is going to be one or more permutations tacked onto the existing known permutation at that point in the calculation. This returned value is used as the accumulator when examining the permutations that involve picking the car of the right side instead.

As far as effeciency, this loop executes once for every element in every permutation. Nothing fancy there, but it will do it in a memory stack no deeper than the sum of the length of the inputs.

(ordered-merge (list 1 2) (list 'a 'b))

;Value 14: ((1 2 a b) (1 a 2 b) (a 1 2 b) (1 a b 2) (a 1 b 2) (a b 1 2))

Overall the shape of how I would approach it is very similar to your second attempt.  Just a few critiques.  Generally cons is the way to build lists. Secondly by mapping up from prior results you are keeping quite a bit extra of data on the stack.

1 ]=> (ordered-merge2 '(1 2 3 4) '(a b c))
(10 0 0)
1 ]=> (ordered-merge3 '(1 2 3 4) '(a b c))
(0 0 1)
1 ]=> (ordered-merge2 '(1 2 3 4 5 6 7 8 9) '(a b c d e f g h))
(170 0 173)
1 ]=> (ordered-merge3 '(1 2 3 4 5 6 7 8 9) '(a b c d e f g h))
(70 0 72)
1 ]=> (ordered-merge2 '(1 2 3 4 5 6 7 8 9) '(a b c d e f g h j k l m o p))
;Aborting!: out of memory
1 ]=> (ordered-merge2 '(1 2 3 4 5 6 7 8 9) '(a b c d e f g h j k l m ))
(2540 1090 3633)
1 ]=> (ordered-merge3 '(1 2 3 4 5 6 7 8 9) '(a b c d e f g h j k l m ))
(860 50 905)
 ]=> (ordered-merge3 '(1 2 3 4 5 6 7 8 9) '(a b c d e f g h j k l m o p q))
(3820 1080 4902)
1 ]=>(ordered-merge3 '(1 2 3 4 5 6 7 8 9) '(a b c d e f g h j k l m o p q r))
;Aborting!: out of memory

Results are timings (cpu-time garbage-collection-time real-time)

Addendum:

I also figure out that my output list was sharing internal list structure where possible.

(define test2 (ordered-merge2 (list 1 2 3) (list 'a 'b 'c)))
(define test (ordered-merge3 (list 1 2 3) (list 'a 'b 'c))
(for-each (lambda (x) (begin (display x) (newline))) test)
(for-each (lambda (x) (begin (display x) (newline))) test2)

Compare the sublists (1 a 2 3 b c) and (a 1 2 3 b c) which in test are elements 2 and 3, and in test2 are 4 and 6. eq? is only true for lists if they are the same object in memory

1 ]=>(eq? (cddr (list-ref test 2)) (cddr (list-ref test 3)))
;Value: #t

1 ]=> (eq? (cddr (list-ref test2 4)) (cddr (list-ref test2 10)))
;Value: #f

1 ]=> (equal? (cddr (list-ref test2 4)) (cddr (list-ref test2 10)))
;Value: #t


Knuth's algorithm M that produces mixed-radix numbers This is the C++ code of my implementation of Knuth's algorithm M that produces mixed-radix numbers:

#include "visit.h"

void algorithmM(vector<int>& m)
{
  m.insert(m.begin(),2);
  const int n=m.size();
  vector<int> a(n,0);
  M2:
  visit(false,a);
  int j=n-1;
  M4:
  if (a[j]==m[j]-1) {a[j]=0;--j;goto M4;}
  if (j==0) return;
  else {a[j]++;goto M2;}
  }
int main()
{
  vector<int> m;
  int i;
  while(std::cin>>i)
  {if(i<0) continue;
   m.push_back(i);
  }
algorithmM(m);
return 0;
}

This is the code of "visit.h":

#include <iostream>
#include <vector>

using std::vector;
using std::cout;

template<class T> void visit(bool first,vector<T>& l)
{
 size_t dt=first?0:1;
 for(typename vector<T>::iterator i=l.begin()+dt;i!=l.end();++i)
cout<<*i;
 cout<<'\n';
}

The C++ code is very close to the Knuth's pseudocode.
And now this is an imperative Haskell implementation using mutable arrays:

import Data.Array.IO
import Control.Monad.State
import Data.IORef

data CountList = CountList {intlist::[Int],count::Int}
lenarr arr = do
         b<-getBounds arr
         return (snd b)

takeInput :: State (String,Int) [Int]
takeInput = do
        (s,count)<-get
        let g=reads s
        if g==[] then return []
        else do
            put (snd(head g),count+1)
            l<-takeInput
            return $ (fst(head g)):l
takeInput2 :: String->CountList
takeInput2 s = let (l,ss)=runState (takeInput) (s,0)
        in CountList l (snd ss)

fillArray :: CountList->IO((IOArray Int Int),(IOArray Int Int))
fillArray l = do
        arr<-newArray (0,(count l)) 0
        x<-nowfill 1 (intlist l) arr
        y<-newArray (0,(count l)) 0
        writeArray x 0 2
        return (x,y)

 where nowfill i l arr = do
             if l==[] then return arr
             else do
                writeArray arr i (head l)
                nowfill (i+1) (tail l) arr
visit ::(IOArray Int Int)->Int->IO ()
visit x i = do
          c<-lenarr x
          if i>c then putStrLn ""
          else do
                a<-readArray x i
                putStr (show a)
                visit x (i+1)

maj :: (IOArray Int Int)->(IOArray Int Int)->Int->IO((IOArray Int Int),Int)
maj m a j = do
        valaj <- readArray a j
        valmj <- readArray m j
        if valaj==valmj-1 then
                  do
                      writeArray a j 0
                      maj m a (j-1)
        else
            return (a,j)
m5 :: (IOArray Int Int)->Int->IO((IOArray Int Int),Int)
m5 a j = if j==0 then
         return (a,j)
     else do
         valaj<-readArray a j
         writeArray a j (valaj+1)
         return (a,j)
algorithmM0 m a = do
    visit a 1
    n<-lenarr m
    (a',j)<-maj m a n
    (a'',j')<-m5 a' j
    if j'==0 then
          return ()
    else
        algorithmM0 m a''
algorithmM = do
    l<-getLine
    let mycountlist = takeInput2 l
    (m,a)<-fillArray mycountlist
    algorithmM0 m a
main :: IO ()
main = algorithmM

I also have and a more functional approach using lists in Haskell which is smaller but I don't want to enlarge the post.

Can you please give me some advice on how to shrink the Haskell code?

I think that the main reason of using a high-level language like Haskell is to write less code but I don't think this happens here so I suppose that I am doing something wrong.

Below is a more idiomatic to Haskell version that still follows your original imperative logic. I know elsewhere on SO you have been shown how to do this efficiently and functionally. I did this to show what could be done if the design requirements remained in place.

This version is 52 lines long with all of the whitespace you see. The original was 84 lines after whitespace between the functions was added.

The entire takeInput design using State was not needed. As shown, map read . words is sufficient. If you are concerned about poor input then a version could quickly be put in place using reads but without State.

case replaces the if usage. This is more idiomatic in my experience and is still very readable. You get pattern matching along the way too.

Note that nowfill was replaced by newListArray and the entire CountList was discarded as unneeded. If it was valuable then fillArray could still use CountList without the need for nowfill. The entire function could be about two lines if Applicative style was used. But that would definitely veer pretty far from the straight forward imperative style.

visit was updated to move the putStrLn call outside of the recursion and uses a helper function instead of the old if/else style. Again, the same exact logic just using a more idiomatic layout.

The calls to uncurry could be removed if m5 and algorithmM0 took tuples instead of separated arguments. This does lend it a more functional flavor but I did not want to disturb the function types where possible.

There are no cute tricks in this code. It is pretty reasonable for even a beginner level Haskell developer to read and understand I think. A C coder unfamiliar with the language would have more difficulty following this version than the original I suspect.

import Data.Array.IO

type AlgoArray = IOArray Int Int -- allows for experimentation with IOUArray or others

lenarr :: AlgoArray -> IO Int  -- simplified type
lenarr arr = getBounds arr >>= return . snd

fillArray :: [Int] -> IO (AlgoArray, AlgoArray)
fillArray ns = do  -- could be Applicative style: (,) <$> newListArray <*> newArray
    x <- newListArray (0, lcount) (2:ns)
    y <- newArray (0, lcount) 0
    return (x, y)
    where
      lcount = length ns

visit :: AlgoArray -> Int -> IO ()
visit x i = lenarr x >>= visit' x i >> putStrLn ""
    where
      visit' x i c
        | i > c     = return ()
        | otherwise = readArray x i >>= putStr . show >> visit' x (i + 1) c

maj :: AlgoArray -> AlgoArray -> Int -> IO (AlgoArray, Int)
maj m a j = do
    valaj <- readArray a j
    valmj <- readArray m j
    maj' valaj valmj
    where
      maj' valaj valmj
        | valaj == (valmj - 1) = writeArray a j 0 >> maj m a (j - 1)
        | otherwise            = return (a, j)

m5 :: AlgoArray -> Int -> IO (AlgoArray, Int)
m5 a 0 = return (a, 0)
m5 a j = readArray a j >>= writeArray a j . (+1) >> return (a, j)

algorithmM0 :: AlgoArray -> AlgoArray -> IO ()
algorithmM0 m a = do
    visit a 1
    v <- lenarr m >>= maj m a >>= uncurry m5
    case v of
      (_, 0)  -> return ()
      (a', _) -> algorithmM0 m a'

algorithmM :: IO ()
algorithmM = getLine >>= fillArray . takeInput >>= uncurry algorithmM0
    where
      takeInput = map read . words

main :: IO ()
main = algorithmM