Time to Next Departure I am trying to find out how many minutes that are remaining until the next train will departure, based on the current system time in Java.

I am so far not familiar with the many aspects of Java, but I have managed to get the program to work in most use cases with a switch statement and while loop, but it feels like I am using too many workarounds to get the code to work as intended.

The next step is that I am trying to complete, the task in a less tedious and more elegant way. I was wondering if anyone could point me in the right direction to how the task could be completed in a similar way but with other features of Java, such as ArrayLists, tables etc. that are suitable for what I am trying to accomplish.

public class TrainDepartures{

public static void main(String[] args) {

    long time = System.currentTimeMillis();
    int initialTime = (int) (time / (1000 * 60) % 60);
    int findNextDeparture = (int) (time / (1000 * 60) % 60);

    boolean foundDeparture = false;

    loop: while (foundDeparture != true) {
        findNextDeparture++;
        switch (findNextDeparture) {
            case 10:
                 System.out.print("Departure in " + (Math.abs(initialTime - 10)) + " min.");
                break loop;
            case 20:
                System.out.print("Departure in " + (Math.abs(initialTime - 20)) + " min.");
                break loop;
            case 30:
                System.out.print("Departure in " + (Math.abs(initialTime - 30)) + " min.");
                break loop;
            case 40:
                System.out.print("Departure in " + (Math.abs(initialTime - 40)) + " min.");
                break loop;
            case 50:
                System.out.print("Departure in " + (Math.abs(initialTime - 50)) + " min.");
                break loop;
            case 52:
                System.out.print("Departure in " + (Math.abs(initialTime - 52)) + " min.");
                break loop;
            case 55:
                System.out.print("Departure in " + (Math.abs(initialTime - 55)) + " min.");
                break loop;
            case 56:
                System.out.print("Departure in " + (Math.abs(initialTime - 56)) + " min.");
                break loop;
            case 57:
                System.out.print("Departure in " + (Math.abs(initialTime - 57)) + " min.");
                break loop;
}
}
}
}

You have a bug

Don't run this code when the current time is 58 or 59 minutes. The loop will never end. You can fix that by adding a modulo in the switch statement:

switch (findNextDeparture % 60) {

This way, it will be ok even if findNextDeparture overflows, you will still get the correct time until the next departure. For example starting from 58, 70 % 60 will match case 10:, giving you 70 - 58 = 12 minutes.

Remove the duplication

In every case statement you do essentially the same thing. You could reduce that to this:

switch (findNextDeparture) {
    case 10:
    case 20:
    case 30:
    case 40:
    case 50:
    case 52:
    case 55:
    case 56:
    case 57:
        System.out.print("Departure in " + Math.abs(initialTime - findNextDeparture) + " min.");
        break loop;
}

Simplify calculations

You're setting findNextDeparture to the same value as initialTime. Instead of spelling out that long expression, it would be better to reuse it:

int initialTime = (int) (time / (1000 * 60) % 60);
int findNextDeparture = initialTime;

Since findNextDeparture starts from the same value as initialTime, and it always increases, you don't need Math.abs(initialTime - findNextDeparture), you can do simply findNextDeparture - initialTime for the same effect.

Improving the implementation

First of all, as others have suggested, the main logic of finding the next departure date should be extracted to its own method, and return the minutes to next departure instead of printing it. Consider this signature:

int findMinutesToNextDeparture(int initialTime) { ... }

This transformation helps with the while loop too: instead of using the loop label, you could just return. Simpler, and more natural, because labels are not a usual practice in Java, and it probably indicates code smell. This transformation helps with another thing: it makes the method testable, see more on that later.

Second, the while loop, the switch, the list of case statements... Not cool! For one thing, stepping through values one by one, sounds silly, doesn't it. If the values are in a list, you could make bigger jumps than doing it 1 by 1. But there's something even better: binary search!

Consider this implementation:

public static int findMinutesToNextDeparture(int initialTime) {
    List<Integer> times = Arrays.asList(10, 20, 30, 40, 50, 52, 55, 56, 57);
    // index of the search key, if it is contained in the array;
    // otherwise, (-(insertion point) - 1).
    // The insertion point is defined as the point at which
    // the key would be inserted into the array:
    // the index of the first element greater than the key, or a.length
    // if all elements in the array are less than the specified key.
    // Note that this guarantees that the return value will be >= 0
    // if and only if the key is found.
    int index = Collections.binarySearch(times, initialTime);
    final int nextDeparture;
    if (-index > times.size()) {
        nextDeparture = times.get(0) + 60;
    } else if (index < 0) {
        nextDeparture = times.get(-index - 1);
    } else {
        nextDeparture = times.get(index + 1);
    }
    return nextDeparture - initialTime;
}

Note that the times list would be probably better as a parameter. I left it inside the method just for the sake of this example. Now, the logic looks quite tricky. I included the JavaDoc of Collections.binarySearch for reference and because it's kind of hopeless to remember this stuff.

Keep in mind that the list must be sorted for binary search to work. If you make a mistake when typing the elements like this it, it will be broken:

List<Integer> times = Arrays.asList(10, 30, 20, 40, 50, 52, 55, 56, 57);

To make sure you cannot make mistakes, you can wrap the list inside a sorted collection, and then again in a random access list like this:

List<Integer> times = new ArrayList<>(new TreeSet<>(Arrays.asList(10, 30, 20, 40, 50, 52, 55, 56, 57)));

Unit testing

I'll be honest: getting the binary search technique right was not easy. It helped that I already had unit tests for the original working implementation. Before you refactor something, make sure you have solid unit tests. This way you can protect yourself from accidentally breaking your working code.

@Test
public void testMinutesBeforeFirst() {
    assertEquals(10, TrainDepartures.findMinutesToNextDeparture(0));
    assertEquals(7, TrainDepartures.findMinutesToNextDeparture(3));
}

@Test
public void testMinutesInBetween() {
    assertEquals(7, TrainDepartures.findMinutesToNextDeparture(13));
    assertEquals(7, TrainDepartures.findMinutesToNextDeparture(43));
}

@Test
public void testExactMatches() {
    assertEquals(2, TrainDepartures.findMinutesToNextDeparture(50));
    assertEquals(3, TrainDepartures.findMinutesToNextDeparture(52));
}

@Test
public void testMinutesBeyondLast() {
    assertEquals(12, TrainDepartures.findMinutesToNextDeparture(58));
    assertEquals(11, TrainDepartures.findMinutesToNextDeparture(59));
    assertEquals(9, TrainDepartures.findMinutesToNextDeparture(61));
}


Does using SharedPreferences drain resources? I have created class that uses SharedPreferences to store and retrieve application settings. Is it good idea to access SharedPreferences each time I need parameters? Will it drain system resources?

class MqttSettings
{

    public static final String ADDRRESS ="address";
    public static final String PORT ="port";
    public static final String USERNAME ="username";
    public static final String PASSWORD ="password";
    public static final String CLIENTID ="clientid";

    public  SharedPreferences preferences;

    public  void setAddress(String value) {
        preferences.edit().putString(ADDRRESS, value ).commit();
    }
    public  void setPort(int value) {
        preferences.edit().putInt(PORT, value ).commit();
    }
    public  void setUsername(String value) {
        preferences.edit().putString(USERNAME, value ).commit();
    }
    public  void setPassword(String value) {
        preferences.edit().putString(PASSWORD, value ).commit();
    }

    public  void setClientId(String value) {
        preferences.edit().putString(CLIENTID, value ).commit();
    }

    public  String getAddress() {
        return preferences.getString(ADDRRESS,"192.168.1.8");
    }
    public  int getPort() {
        return preferences.getInt(PORT,8883);
    }
    public  String getUsername() {
        Log.d(Utils.LOGTAG,"getting username");
        String r = null;
        try {
            r= preferences.getString(USERNAME,"");
            Log.d(Utils.LOGTAG,"getting username preferences");
        } catch (Exception e) {
            Log.d(Utils.LOGTAG,"got exception "+e.getMessage());
            e.printStackTrace();
        }
        Log.d(Utils.LOGTAG,"getting username return");
        return r;
    }
    public  String getPassword() {
        Log.d(Utils.LOGTAG,"getting password");
        return preferences.getString(PASSWORD,"");
    }
    public  String getClientId() {
        return preferences.getString(CLIENTID,"");
    }

    public  String getUri()
    {
        String  uri ="tcp://";
        uri=uri+getAddress()+":"+getPort();
        Log.d(Utils.LOGTAG,uri);
        return uri;

    }

    public MqttSettings(SharedPreferences preferences)
    {
        this.preferences = preferences;
    }

}

Accessing SharedPreferences takes some definite amount of time because they are written and read from an actual file. So if you call this code too often from your mainThread, you will end up with ApplicationNotResponding errors and a bad user experience. The point is to call this on a background Thread (java) or coroutine(kotlin). The same goes for accessing the network, reading and writing to the database, long calculations etc

https://developer.android.com/training/articles/perf-anr


Conditional aggregate whilst constructing array I have the following snippet of code which gets a load of time intervals (every hour, every 2 hours etc), then groups the results for each of the intervals (e.g, the max value for each hour).  This data needs to go into an array (which will be sent to a Chart).

The user can select if they wish to group my Min, Max or Average.

At the moment I have a switch statement that has the same code in each branch except the Max/Min/Average bit at the end.

Can I avoid duplicating the code whilst still having the Min/Max/Average conditional?

The code as it is:

var vm = new MyViewModel();
var times = getTimeIntervals();
var results = getResults();

vm.GroupedData =
   times.Select(x =>
      new object[] {
         x.Start.ToInstant().Ticks,
         results.Data
             .Where(i => x.Intersects(i.Date))
             .Select(v => v)
             .DefaultIfEmpty()
             .Max(i => i == null ? 0m : decimal.Parse(i.Value))
             //.Min(i => i == null ? 0m : decimal.Parse(i.Value))
             //.Average(i => i == null ? 0m : decimal.Parse(i.Value))
   });

The code I'd like:

vm.GroupedData =
           times.Select(x =>
              new object[] {
                 x.Start.ToInstant().Ticks,
                 results.Data
                     .Where(i => x.Intersects(i.Date))
                     .Select(v => v)
                     .DefaultIfEmpty()
                     .If(Avg)
                         .Average(i => i == null ? 0m : decimal.Parse(i.Value))
                     .ElseIf(Max)
                         .Max(i => i == null ? 0m : decimal.Parse(i.Value))
                     .ElseIf(Min)
                         .Min(i => i == null ? 0m : decimal.Parse(i.Value))
           });

First of all you can simplify your Code to:

vm.GroupedData =
   times.Select(time =>
      new { Ticks = time.Start.ToInstant().Ticks,
            Aggregate = results.Data
             .Where(data => time.Intersects(data.Date))
             .Select(data => data == null ? 0m : decimal.Parse(data.Value))
             .DefaultIfEmpty()
             .Max()
             //.Min()
             //.Average()
   });

1.) Remove the Select(v => v)

The Select doesn't actually do anythin in your case, you will get the same result without it. Select is used to project the single elements in your result into another format. E.g. get only a list of the Values insted of the whole object.

2.) Remove the DefaultIfEmpty()

With the DefaultIfEmpty you get a List containing only one "null" element. which would produce the same result as calling Max, Min or Average on an empty list.

3.) Transform the Results before using aggregates

Transform your result list to a list of all the values as decimal with a Select instead of doing this in each of the subfunctions.

4.) Give x and i meaningful Names

5.) Use annonymous Type instead of object array or even better create own type for the tick-aggregate pair

To your actual question: Instead of calculating the Aggregate  you could store the IEnumerable in your result object and then do a second select on that result with your desired aggregate function. As long as you store the query as IEnumerable the query shouldn't be executed until you want to access the actual values of the result.

var myDataRowsPerTime = times.Select(time =>
      new { Tick = time.Start.ToInstant().Ticks,
            DataRow = results.Data
             .Where(data => time.Intersects(data.Date))
             .Select(data => data == null ? 0m : decimal.Parse(data.Value))
             .DefaultIfEmpty()
   });

switch(aggregationMethod)
{
    case AggregationMethod.Avg:
        vm.Groupdata = myDataRowsPerTime.Select(item =>
             new { Tick = item.Tick, Aggregate = item.DataRow.Avg() });
        break;
   case AggregationMethod.Max:
         ...

You should define a own class for the last result, there maybe a problem with accessing the Data of the annonymous Type in the viewmodel. The viewmodel doesn't know the structure of this annonymous type.


Fastest fill memory with specified 64-bit value I need to fast fill a memory block in C#, so I wrote something like this (in Free Pascal Compiler + Lazarus in 64 bit mode):

//  ECX = Ptr64
//  EDX = Count
//  R8  = Value

PROCEDURE Fill64 (VAR Ptr64; Count: QWord; Value: QWord);
BEGIN
  ASM
    PUSH    RDI
    MOV     RDI, RCX    // Destination Index = Ptr64
    MOV     RAX, R8     // Accumulator = Value
    MOV     RCX, RDX    // Counter Register = Count
    TEST    RCX, RCX    // If RCX is 0, set ZF (zero flag)
    JZ      @Exit       // Exit if ZF is set
    REP     STOSQ       // Fill memory using 64 bit value from RAX register
  @Exit:
    POP     RDI
  END;
END;

And I use it from C#:

[DllImport ("MemUtil64.dll", EntryPoint = "Fill64", CallingConvention = CallingConvention.Cdecl)]
private static extern unsafe void Fill64 (void* ptr, ulong count, ulong value);

If this code okay, and fastest possible?

Unfortunately the answer is CPU-dependent. For example here is Android's memset implementation: it uses rep stosq for some CPUs and a different more complicated implementation (avoiding rep stosq) for others.

For further details, on Intel CPUs, refer to the "Enhanced REP MOVSB and STOSB operation (ERMSB)" section of the Intel® 64 and IA-32
Architectures
Optimization Reference Manual.


Unit-testing an adapter I want to create a class ClientSocket with PHP, which is an adapter of the fsockopen, fread, and fwrite.

Class ClientSocket extends SenderAdapterAbstract
{
    private $s = null;

    public function __construct($host, $port)
    {
        $this->s = fsockopen($host, $port);
    }

    // Abstract method concrete implementation
    public function send($message)
    {
        fwrite($this->s, $message);

        $result = fgets($this->s);
        fclose($this->s);
    }
}

Should I test this class?  A unit test is not really one if it talks to the network.

Here is some rules about unit testing (I totally agree with them):

A test is not a unit test if:

It talks to the database
It communicates across the network
It touches the file system
It can't run at the same time as any of your other unit tests
You have to do special things to your environment (such as editing config files) to run it.

a unit test is not really one if it talks to the network...

I am not so sure what you meant by that. It seems like you are trying to follow exactly what someone may have said in a book or a talk. Unit testing is all about testing a 'Model'. And the assumptions are that the Model is isolated from any other models in the overall project.

In your case the Unit Test for the ClientSocket would be called ClientSocketTest.

Class ClientSocketTest extends PHPUnit_Testcase
{
    private $s = null;

    public function testSend()
    {
        $cs = new ClientSocket();
        $cs->send("My Message");
        $result = $cs->result;
        $this->assert("Everything's Good", $result);
    }
}

I noticed that your send method has the variable $result. For Unit Testing to be successful a method should return something. The purpose of Unit Testing is to see whether the given input gives you the expected output. Don't overcomplicate it!

Start with PHPUnit... and stick to it! :)


Regular Expression in Javascript to test a string of 1s and 0s I have a string of four bits which represent true/false values. There are only seven valid  options:

1010
1110
0000
1101
1001
1000
0101

There are three options which could potentially be selected which are not valid and that I want to check for before I proceed with some other code. These are:

0110
0100
0010

I want to do this with as little code as possible thus having one regex to test all three conditions. My question is if this is a correct regex to accomplish this test. It seems to work, but I am not a regex expert, and have to be sure in this case.

 if (!/0(10|01|11)0/.test(precode)) {
     //do some code
 }

Why not simply test the valids?

if((/0110|0100|0010/).test(precode))

Seems more readable to me.


ValueAnimator for IOS - Did I butcher it up? I am trying to make an IOS tool similar to Android's ValueAnimator... more or less

This is useful for situations where you simply want a value to "animate" over time, but rather than necessarily attaching that value to some view property, it is available to use as you wish.  Imagine applying it to an audio filter, for example.

It works like a Timer/NSTimer except that is passes a value back to the selector which behaves differently depending on the arguments sent to the initializer.

Concerns:

Is there a way to tidy up the numerous functions into something like a dictionary? (I'm not that advanced at Swift)

Is there a way of accomplishing the same functionality -- passing a "listener" function in on initialization which will be called back with the animated value as an argument -- but without using a selector? ... I ask because of the nasty "MyNSDoubleObject" workaround.

Any other improvements... this is my first question on here.

Code:

import Foundation

// this class is a work-around for the fact that we can't pass a raw value (int, double, etc.) through a selector -- must be object
class MyNSDoubleObject: NSObject {
    var val: Double

    init(val: Double) {
        self.val = val
    }
}

class ValueAnimator : NSObject {

    // args
    private let sampleRate: Int
    private let functionType: FunctionType
    private let selector: Selector
    private let target: AnyObject

    // computed from args
    private let maxIterations: Int
    private let timeInterval: Double
    private var X_currentRepIndex: Int = 0 // domain
    private var F_of_X = MyNSDoubleObject(val: 0.0) // range

    // other
    private var timer = Timer()

    enum FunctionType {
        case SINE_WAVE_FROM_0_TO_1_TO_0
        case SINE_WAVE_FROM_0_TO_1
        case SINE_WAVE_FROM_1_TO_0
    }

    // Public functions
    init(durationInSeconds: Int, sampleRate: Int, functionType: FunctionType, selector: Selector, target: AnyObject) {
        self.sampleRate = sampleRate
        self.maxIterations = durationInSeconds * sampleRate
        self.timeInterval = 1.0/Double(sampleRate)
        self.functionType = functionType
        self.selector = selector
        self.target = target
    }

    func start() {
        timer = Timer.scheduledTimer(timeInterval: timeInterval, target: self, selector: (#selector(timerCallback)), userInfo: nil, repeats: true)
    }

    // "Private" functions
    @objc func timerCallback() {

        switch functionType {
        case .SINE_WAVE_FROM_0_TO_1_TO_0:
            F_of_X.val = sine010(x: X_currentRepIndex)
        case .SINE_WAVE_FROM_0_TO_1:
            F_of_X.val = sine01(x: X_currentRepIndex)
        case .SINE_WAVE_FROM_1_TO_0:
            F_of_X.val = sine10(x: X_currentRepIndex)
        }

        X_currentRepIndex += 1

        _ = target.perform(selector, with: F_of_X)

        if X_currentRepIndex == (maxIterations+1) {
            timer.invalidate()
        }
    }

    private func sine010(x: Int) -> Double {
        return (-cos(2*Double.pi * (Double(x)/Double(maxIterations)))+1)/2
    }

    private func sine01(x: Int) -> Double {
        return (-cos(Double.pi * (Double(x)/Double(maxIterations)))+1)/2
    }

    private func sine10(x: Int) -> Double {
        return (cos(Double.pi * (Double(x)/Double(maxIterations)))+1)/2
    }

}

Is there a way to tidy up the numerous functions into something like a dictionary?

Sure, there are various options. But first the functions should be made independent of maxIterations, i.e. the computation

Double(currentIteration) / Double(maxIterations)

should be done in the caller so that the functions simplify to

private func sine010(x: Double) -> Double {
    return (-cos(2 * .pi * x) + 1)/2
}

thus avoiding code duplication. Note also the use of the “implicit member expression” .pi – the type Double is automatically inferred from the context.

Now you could define a dictionary mapping a function types to functions. The functions can be embedded directly as closures instead of defining global functions:

let functionDict : [FunctionType: (Double) -> Double] = [
    .SINE_WAVE_FROM_0_TO_1_TO_0: { (1 - cos(2 * .pi * $0)) / 2 },
    .SINE_WAVE_FROM_0_TO_1: { (1 - cos(.pi * $0)) / 2 },
    .SINE_WAVE_FROM_1_TO_0: { (1 + cos(.pi * $0)) / 2 },
]

The disadvantage is that you are now responsible to update the dictionary if new function types are added.

A perhaps better alternative is to make the function a computed property of the function type enumeration:

enum FunctionType {
    case SINE_WAVE_FROM_0_TO_1_TO_0
    case SINE_WAVE_FROM_0_TO_1
    case SINE_WAVE_FROM_1_TO_0

    var f: (Double) -> Double {
        switch self {
        case .SINE_WAVE_FROM_0_TO_1_TO_0: return { (1 - cos(2 * .pi * $0)) / 2 }
        case .SINE_WAVE_FROM_0_TO_1: return { (1 - cos(.pi * $0)) / 2 }
        case .SINE_WAVE_FROM_1_TO_0: return { (1 + cos(.pi * $0)) / 2 }
        }
    }
}

Now everything is in “one place” and the compiler can check the exhaustiveness of the switch statement.

A disadvantage of all the above definitions is that they can not be extended: There is no way that a user can define its own interpolation function and pass it to the value animator.

So what I would really do is to define a struct as the function wrapper, with static properties for predefined functions. I am calling it Interpolator now (resembling the Android TimeInterpolator).

class ValueAnimator {

    struct Interpolator {
        // A function mapping [0, 1] to [0, 1].
        let f: (Double) -> Double

        // Predefined interpolation functions
        static let sineWaveFrom0To1To0 = Interpolator(f: { (1 - cos(2 * .pi * $0)) / 2 } )
        static let sineWaveFrom0To1 = Interpolator(f: { (1 - cos(.pi * $0)) / 2 } )
        static let sineWaveFrom1To0 = Interpolator(f: { (1 + cos(.pi * $0)) / 2 } )
    }

    // ...
}

Note also that I switched to lower camel case for the property names, which is the standard for all Swift identifiers except for types.

Now a user can easily add more interpolation functions, e.g.

extension ValueAnimator.Interpolator {
    static let linear = ValueAnimator.Interpolator(f: { $0 } )
}

  Is there a way of accomplishing the same functionality ... but without using a selector? ... I ask because of the nasty "MyNSDoubleObject" workaround.

First: The wrapper object is not needed even if you use selectors: Swift automatically wraps values in Objective-C compatible types if necessary. Which means that you can pass a floating point value through the selector:

let value: Double = ...
target.perform(selector, with: value)

and the receive can (conditionally) cast it back to a Double:

@objc func animate(obj: AnyObject) {
    guard let value = obj as? Double else { return }
    print(value)
}

So the MyNSDoubleObject workaround is not needed.

But it becomes much simpler if you replace the target/selector method by a simpler callback with a closure. Similarly for the local timer: With a block-based timer, the timer callback need not be Objective-C compatible, and the ValueAnimator class does not have to subclass NSObject anymore.

Some minor remarks: The

private let sampleRate: Int

property is not needed. Instead of initializing with an (inactive) timer

private var timer = Timer()

I would use an optional:

private var timer: Timer?

X_currentRepIndex and F_of_X to not follow the Swift naming conventions, the latter name is quite non-descriptive.

Putting it all together, the ValueAnimator class could look like this:

class ValueAnimator {

    struct Interpolator {
        // A function mapping [0, 1] to [0, 1].
        let f: (Double) -> Double

        // Predefined interpolation functions
        static let sineWaveFrom0To1To0 = Interpolator(f: { (1 - cos(2 * .pi * $0)) / 2 } )
        static let sineWaveFrom0To1 = Interpolator(f: { (1 - cos(.pi * $0)) / 2 } )
        static let sineWaveFrom1To0 = Interpolator(f: { (1 + cos(.pi * $0)) / 2 } )
    }

    // args
    private let interpolation: Interpolator
    private let callback: (Double) -> Void

    // computed from args
    private let maxIterations: Int
    private let timeInterval: Double
    private var currentIteration: Int = 0

    // Other properties
    private var timer: Timer?

    init(durationInSeconds: Int, sampleRate: Int, interpolation: Interpolator,
         callback: @escaping (Double) -> Void) {
        self.maxIterations = durationInSeconds * sampleRate
        self.timeInterval = 1.0 / Double(sampleRate)
        self.interpolation = interpolation
        self.callback = callback
    }

    func start() {
        timer = Timer.scheduledTimer(withTimeInterval: timeInterval, repeats: true) { (timer) in
            let val = Double(self.currentIteration)/Double(self.maxIterations)
            self.callback(self.interpolation.f(val))
            self.currentIteration += 1
            if self.currentIteration > self.maxIterations {
                self.stop()
            }
        }
    }

    func stop() {
        timer?.invalidate()
        timer = nil
    }
}

and a sample usage could look like this:

class ViewController: ViewController {

    var valueAnimator: ValueAnimator?

    override func viewDidLoad() {
        valueAnimator = ValueAnimator(durationInSeconds: 2, sampleRate: 2,
                                      interpolation: .sineWaveFrom0To1To0)
        { [weak self] value in
            guard let self = self else { return }

            // Do something with value ...
        }
        valueAnimator?.start()
    }

}

Note the use of weak self in the closure to avoid a reference cycle.

Further thoughts:

Since  ValueAnimator is a “pure Swift” class now it can be made generic to support other value types, such as integers.
Finally: If the goal is to animate visual elements: don't reinvent the wheel, use Core Animation.


Calculate 8 hours business day in a weekdays only I am fairly new into asp.net MVC c# I am working on request holiday app where an employee can request holidays. The problem is the app works fine, however,  I am trying to set 8 hours a day when an employee requests a holiday I have scratching my head from 2 days already seen every topic in StackOverflow and here and still can't get to work if anyone has any suggestions it will be much appreciated.
Here is a snippet of the calculation of the working days excluding weekends. Thank you in advance.

 public static class DateHelpers
    {
        public static int DaysBetweenExcludingWeekends(DateTime startDate, DateTime endDate)
        {

            var days = 0;
            while (startDate <= endDate)
            {
                if (startDate.DayOfWeek != DayOfWeek.Saturday && startDate.DayOfWeek != DayOfWeek.Sunday)

                {
                        days++;

                }

                startDate = startDate.AddDays(1);
            }

            return days;
        }

    }

You can throw an error if someone is trying to set a work day to a day that should be a day off. Then you just need to handle this error.


Escape the Trolls, or just walking through a maze Clojure beginner, looking to improve this code I submitted for Escape the Trolls, a Weekly Programming challenge over in reddit. No trolls yet, just walking through a maze for now.

(ns ctrollmaze.core
  (:gen-class)
  (:require [clojure.string :as str]))

(def sample-maze
"#####################################
# #       #       #     #         # #
# # ##### # ### ##### ### ### ### # #
#       #   # #     #     # # #   # #
##### # ##### ##### ### # # # ##### #
#   # #       #     # # # # #     # #
# # ####### # # ##### ### # ##### # #
# #       # # #   #     #     #   # #
# ####### ### ### # ### ##### # ### #
#     #   # #   # #   #     # #     #
# ### ### # ### # ##### # # # #######
#   #   # # #   #   #   # # #   #   #
####### # # # ##### # ### # ### ### #
#     # #     #   # #   # #   #     #
# ### # ##### ### # ### ### ####### #
# #   #     #     #   # # #       # #
# # ##### # ### ##### # # ####### # #
# #     # # # # #     #       # #   #
# ##### # # # ### ##### ##### # #####
# #   # # #     #     # #   #       #
# # ### ### ### ##### ### # ##### # #
# #         #     #       #       # #
#X###################################")

(defrecord Maze [width
                 height
                 grid])

(defn flatten-coords
  [maze x y]
  (let [width (:width maze)]
    (+ x (* y width) y)))

(defn token-at
  [maze x y]
  (let [grid (:grid maze)]
    (nth grid (flatten-coords maze x y))))

(defn is-token?
  [maze x y given-token]
  (let [token (token-at maze x y)]
    (= given-token token)))

(defn is-wall?
  [maze x y]
  (is-token? maze x y \#))

(defn is-walkable?
  [maze x y]
  (some (partial is-token? maze x y) '(\space \X)))

(defmulti gen-maze (fn [strategy] (:strategy strategy)))

(defmethod gen-maze :static
  [strategy]
  (->Maze (:width strategy) (:height strategy) (into [] (seq sample-maze))))

(defn move-player
  [direction player]
  (case direction
    "a" (assoc player :dir \< :x (dec (:x player)))
    "d" (assoc player :dir \> :x (inc (:x player)))
    "w" (assoc player :dir \^ :y (dec (:y player)))
    "s" (assoc player :dir \v :y (inc (:y player)))
    player))

(defn gen-static-maze
  []
  (gen-maze { :strategy :static :width 37 :height 23 }))

(defn place-player
  ([maze player] (place-player maze player player))
  ([maze player' player]
   (println player' player)
   (let [{:keys [grid width height]} maze
         x' (:x player')
         y' (:y player')
         dir' (:dir player')
         x (:x player)
         y (:y player)]
     (->Maze width height (assoc grid
                                 (flatten-coords maze x y) \space
                                 (flatten-coords maze x' y') dir')))))

(defn pretty-print
  [maze]
  (let [grid (:grid maze)]
    (println (str/join grid))))

(defn is-game-finished?
  [maze]
  (->> (:grid maze)
       (some #{\X})
       (boolean)
       (not)))

(defn play-maze
  []
  (loop [player { :dir \> :x 1 :y 1 }
         maze (place-player (gen-static-maze) player)
         input-dir nil]
    (let [player' (move-player input-dir player)]
      (if (is-walkable? maze (:x player') (:y player'))
        (let [maze' (place-player maze player' player)]
          (pretty-print maze')
          (if (is-game-finished? maze')
            true
            (recur player' maze' (read-line))))
        (recur player maze (read-line))))))

(defn -main
  "I don't do a whole lot ... yet."
  [& args]
  (when (play-maze)
    (println "Congratulations, you won!")))

Version 1.1: No longer does the output lag behind one move.

Looks clean enough IMO, separate functions, a record for the maze -
perhaps create one for the player too?

When run from the terminal the (println player' player) is just
noise, I'd remove it.  Also, the maze should be printed once before
any commands are entered.
A lot of the lets can be inlined as they're only used once.
I'd perhaps pass coordinates in pairs instead of single numbers, then
you save a parameter every time and keep those two values that belong
together in one place.  OTOH the update code will be a little bit more
complex.
For is-game-finished? the threading macro ->> looks like overkill,
it's not like the expression is very complicated.
The return value of play-maze doesn't really matter, does it, so
maybe just drop the true and use when instead.

Perhaps add a way to clear the screen before printing the maze, then
at least on the terminal it's less cluttered.
Perhaps also switch to reading single characters so you don't have to
constantly press the Enter key.


Ruby implementation for Hacker Rank connected cells in a grid https://www.hackerrank.com/challenges/connected-cell-in-a-grid/problem

problem statement:

  Consider a matrix where each cell contains either a 1 or a 0.  Any
  cell containing a 1 is called a filled cell.  Two cells are said to be
  connected if they are adjacent to each other horizontally,
  vertically, or diagonally. In the following grid,  all cells marked X
  are connected to the cell marked Y.

  XXX
  XYX
  XXX
  If one or more filled cells are also connected, they
  form a region. Note that each cell in a region  is connected to zero
  or more cells in the region but  is not necessarily directly connected
  to all the other cells in the region.

  Given an  matrix, find and print the number of cells in the largest
  region  in the matrix. Note that there may be more than one region in
  the matrix.

  For example, there are two regions in the following  matrix.  The
  larger region at the top left contains  cells.  The smaller one at the
  bottom right contains .

  110
  100
  001

my solution:

# Complete the connectedCell function below.
def connectedCell(matrix)
    highest_count = 0
    visited = {}

    (0...matrix.length).each do |i|
        (0...matrix[0].length).each do |j|
        next if visited[[i,j]]
            if matrix[i][j] == 1
                res = get_area_count([i,j], matrix)
                if res[0] > highest_count
                    highest_count = res[0]
                end

                visited = visited.merge(res[1])
            end
        end
    end

    highest_count
end

def get_area_count(pos, matrix)
    q = [pos]
    visited = {}
    count = 0
    while q.length > 0
        tile_pos = q.shift
        if !visited[tile_pos]
            count += 1
            visited[tile_pos] = true
            q += nbrs(tile_pos, matrix)
        end
    end

    return [count, visited]
end

def nbrs(pos, matrix)
    right = [pos[0], pos[1] + 1]
    left = [pos[0], pos[1] - 1]
    top = [pos[0] + 1, pos[1]]
    bottom = [pos[0] - 1, pos[1]]
    top_right = [top[0], right[1]]
    bottom_right = [bottom[0], right[1]]
    top_left = [top[0], left[1]]
    bottom_left = [bottom[0], left[1]]
    positions = [right, left, top, bottom, top_right, bottom_right, top_left, bottom_left]
    positions.select{|npos| in_bounds?(npos, matrix.length, matrix[0].length) && matrix[npos[0]][npos[1]] == 1}
end

def in_bounds?(pos, m, n)
    pos[0] >= 0 && pos[0] < m && pos[1] >= 0 && pos[1] < n
end

thought process:
My thought was to iterate through each cell in the matrix and then if it was a 1 I would do a depth-first traversal to find all other cells that had a 1and were connected to the parent cell. then I would add 1 to the count whenever I visited a cell and add it to the visited hash so that it wouldn't be added to the count. I added helper methods for in_bounds? and nbrsmostly for better readability in the get_area_count method(which is the dfs implementation). The nbrs method is pretty verbose, but I kept it that on purpose because I'm preparing for technical interviews, where accuracy is important, and I thought actually listing out each direction would help in debugging / explaining it to the interviewer.

I really like clear and speaking method names, I quickly tried to refactor the first method but have had not much time, I hope this helps anyway.

I have not understand the other initialization of the visited hash in the area_count method (yet).

The new neighbors method indeed looks some kind of overloaded with information, no good idea yet how to change it, if needed at all as you described.

require 'json'
require 'stringio'

def connectedCell(matrix, visited = {})
    (0...matrix.length).each do |row|
        (0...matrix[0].length).each do |col|
        next if visited[[row, col]]
            if matrix[row][col] == 1
              return update_count_stats(row, col, matrix, visited)
            end
        end
    end
end

def update_count_stats(row, col, matrix, visited)
    result = area_count([row, col], matrix)
    visited.merge(result[1])
    result[0] > 0 ? result[0] : 0
end

def area_count(pos, matrix)
    q = [pos]
    visited = {}
    count = 0
    while q.length > 0
        tile_pos = q.shift
        if !visited[tile_pos]
            count += 1
            visited[tile_pos] = true
            q += neighbors(tile_pos, matrix)
        end
    end

    return [count, visited]
end

def neighbors(pos, matrix)
    right = [pos[0], pos[1] + 1]
    left = [pos[0], pos[1] - 1]
    top = [pos[0] + 1, pos[1]]
    bottom = [pos[0] - 1, pos[1]]

    top_right = [top[0], right[1]]
    bottom_right = [bottom[0], right[1]]
    top_left = [top[0], left[1]]
    bottom_left = [bottom[0], left[1]]

    positions = [right, left, top, bottom, top_right, bottom_right, top_left, bottom_left]
    positions.select{|npos| in_bounds?(npos, matrix.length, matrix[0].length) && matrix[npos[0]][npos[1]] == 1}
end

def in_bounds?(pos, number_of_columns, number_of_rows)
    pos[0] >= 0 && pos[0] < number_of_columns && pos[1] >= 0 && pos[1] < number_of_rows
end

number_of_rows = 4 # just for testing, was gets.to_i
number_of_columns = 4 # just for testing, was gets.to_i
matrix = Array.new(number_of_rows)

# sample input
# 1 1 0 0
# 0 1 1 0
# 0 0 1 0
# 1 0 0 0

number_of_rows.times do |i|
    matrix[i] = gets.rstrip.split(' ').map(&:to_i)
end
result = connectedCell matrix
puts result
```


Implementing Array#uniq in Ruby I've implemented a working version of Array#uniq in Ruby to answer the question here: http://www.rubeque.com/problems/like-a-snowflake, but I'd like to see if there's ways to clean up my code or some better practices I should follow?  This is my version:

class Array
  def uniq(&block)
    objs = []
    yield_vals = []
    self.each do |obj|
      yield_val = yield obj
      unless yield_vals.include?(yield_val)
        yield_vals.push(yield_val)
        objs.push(obj)
      end
    end
    objs
  end
end

For one, I'd use the << operator rather than push just out of convention.

Second, you still have other Array/Enumerable methods at your disposal. So there's no need to create a new array and add unique items to it. Try filtering the array instead.

Update: Whoo boy, I was still asleep when I wrote that first answer. In my defense it was early

You can actually just do

def uniq(&block)
  group_by(&block).values.map(&:first)
end

As for my previous answer... well, I did say it could be done better. Just pretend it didn't happen.


Bookmarking directories When working on a large codebase, I often need to switch between deeply nested directories quickly. To make my life easier, I came up with a bookmarking script.

#!/bin/bash

bm_list() {
    for i in "${!BOOKMARKS[@]}"; do
        echo -e $i'\t'"${BOOKMARKS[$i]}" ;
    done
}

bm_go() {
    cd "${BOOKMARKS[$1]}"
}

bm_add() {
    for i in "${!BOOKMARKS[@]}"; do
        [[ "$1" == ${BOOKMARKS[$i]} ]] && return
    done
    BOOKMARKS+=("$1")
}

bm_clear() {
    unset BOOKMARKS
}

bm_delete() {
    unset BOOKMARKS[$1]
}

bm_compact()
{
    local SIFS="$IFS"; IFS=
    BOOKMARKS=("${BOOKMARKS[@]}")
    IFS="$SIFS"
}

bm_load() {
    local bmfile=${1:-~/.bookmarks}
    local SIFS="$IFS"; IFS=$'\n'
    bm_clear
    while read -d ''; do BOOKMARKS+=("$REPLY"); done < "$bmfile"
    IFS="$SIFS"
}

bm_save() {
    local bmfile=${1:-~/.bookmarks}
    printf '%s\0' "${BOOKMARKS[@]}" > "$bmfile"
}

bm_help() {
    cat <<-EOF
bm: bookmark directories
Usage:
    bm                   bookmark current directory
    bm -l
    bm list              list bookmarks
    bm index
    bm -g index
    bm go index          cd to bookmark at index
    bm -d index
    bm delete index      delete bookmark at index
    bm -s [filename]
    bm save [filename]   save bookmarks in the file (default ~/.bookmarks)
    bm -L [filename]
    bm load [filename]   load bookmarks from the file (default !/.bookmarks)
    bm -c
    bm clear             remove all bookmarks
    bm -C
    bm compact           compact indices to a sequential list
EOF
}

bm() {
    local command=$1; shift
    case $command in
        -l | list ) bm_list ;;
        -g | go ) bm_go "$1" ;;
        -d | delete ) bm_delete "$1";;
        -s | save ) bm_save "$1" ;;
        -c | clear ) bm_clear ;;
        -C | compact ) bm_compact ;;
        -L | load ) bm_load "$1" ;;
        '') bm_add "$PWD" ;;
        *) [[ "$command" =~ [0-9]+ ]] && bm_go "$command" || bm_help ;;
    esac
}

Some suggestions:

#!/usr/bin/env bash is a very common shebang line these days.
Use More Quotes™:

echo -e $i'\t'"${BOOKMARKS[$i]}" should be echo -e $i'\t'"${BOOKMARKS[$i]}" (or even better, printf '%s\t%s' "$i" "${BOOKMARKS[$i]}")
command=$1 should be command="$1"

In [[ "$command" =~ [0-9]+ ]] you'll want to anchor the regex.
If this is for your own use I don't quite understand the need for both option-style and command-style invocation.
== in Bash is a bit of a historical accident. It won't make any difference as long as you use Bash, but I usually prefer the POSIX = in this case.
It seems bm_compact should run on every change, since it'll be super fast and you avoid changing indexes.
bmfile would be better as either configuration or as a single variable.
"default !/.bookmarks" should probably be "default ~/.bookmarks".
You can set IFS for a single command, such as while IFS=… read …
All the single semicolons are extraneous - you definitely don't need one for echo -e $i'\t'"${BOOKMARKS[$i]}" ;, and you can use

for i in "${!BOOKMARKS[@]}"
do

for loops and conditionals.


RouteContext as constructor parameter or method parameter in ASP.NET vNext I have this simple class where I load an object from the database and store it into the RouteContext.DataTokens because I need to access it through out the request.

In this example the RouteContext is injected via the constructor but I'm not sure that's the right thing to do here, maybe the RouteContextshould be a method parameter instead?

public class RouteResolverTrie : IRouteResolverTrie
{
    private readonly IDocumentStore documentStore;
    private RouteContext context;

    public RouteResolverTrie(RouteContext context, IDocumentStore documentStore)
    {
        this.documentStore = documentStore;
        this.context = context;
    }

    public Trie LoadTrie()
    {
        if(context.DataTokens.ContainsKey("some key")) {
            return context.DataTokens["trie"] as Trie;
        }
        else {
            using(var session = documentStore.OpenSession()) {
                var trie = session.Load<Trie>("trie/1");
                context.DataTokens["trie"] = trie;
                return trie;
            }
        }
    }
}

This is how the routing works in vNext and this is my first entry point. As you can see I use LoadTrie() in the method RouteAsync which in turn uses the RouteContext to store temporary data through out the request. So, the question is, should I instantiate the DefaultRouter with a new RouteResolverTrie that takes the current RouteContext as a constructor parameter or should it be a method parameter?

public class DefaultRoute : IRouter
{
    private readonly IRouter _target;
    private IRouteResolverTrie _routeResolverTrie;

    public DefaultRoute(IRouter target, IRouteResolverTrie routeResolverTrie) {
        _target = target;
        _routeResolverTrie = routeResolverTrie;
    }

    public string GetVirtualPath(VirtualPathContext context)
    {
        return null;
    }

    public async Task RouteAsync(RouteContext context)
    {
        var trie = _routeResolverTrie.LoadTrie();
    }
}

Since you need something that persists for the duration of request, a better place would be to add to RequestContext. So there are couple of ways to handle this more cleanly:

Add a base controller and add additional data to RequestContext:

protected override void Execute(System.Web.Routing.RequestContext requestContext)
{
    // get additional data
    ...

    // add them to routeDate
    requestContext.RouteData.Values.Add(key, value);
    ...

    base.Execute(requestContext);
}

Add your own custom RouteHandler and add additional data to RequestContext:

public class CustomRouteHandler : MvcRouteHandler
{

    protected override IHttpHandler GetHttpHandler(RequestContext requestContext){
           // add to HttpContext
           requestContext.HttpContext.Items["Custom"] = data;

           // or RouteData
           requestContext.RouteData.Values.Add(key, value);
        }
        return base.GetHttpHandler(requestContext);
    }

}

// define route with custom handler
Route customRoute = new Route (
    url: ...,
    defaults: null,
    constraints: ...,
    routeHandler: new CustomRouteHandler()
});

routes.Add(customRoute);


Time of day based timer - run function at certain time, daily I need to call a function exactly at certain hour, daily, my programs runs 24/7.

I wrote the following timer mechanism, which generally works that way:

Calculate time until hour and generate timer with that interval.
When timer elapse, call timer event and generate another 24h timer.
Go back to step 2.

private static void InitTimeBasedTimer(TimeSpan timeToRun, Action funcToRun)
{
    TimeSpan interval = timeToRun.Subtract(DateTime.Now.TimeOfDay);
    double intervalMillis = interval.TotalMilliseconds;
    if (interval.TotalMilliseconds < 0)
    {
        intervalMillis = (new TimeSpan(24, 0, 0)).Add(interval).TotalMilliseconds;
    }
    Timer timer = new Timer(intervalMillis);
    timer.AutoReset = false;
    timer.Elapsed += (sender, e) => TimeBasedCallback(timeToRun, funcToRun);
    timer.Start();
}

public static void TimeBasedCallback(TimeSpan timeToRun, Action funcToRun)
{
    funcToRun();
    TimeSpan interval = timeToRun.Subtract(DateTime.Now.TimeOfDay); //24 h
    double intervalMillis = interval.TotalMilliseconds;
    if (intervalMillis < 0)
    {
        intervalMillis = (new TimeSpan(24, 0, 0)).Add(interval).TotalMilliseconds;
    }
    Timer timer = new Timer(intervalMillis);
    timer.AutoReset = false;
    timer.Elapsed += (sender, e) => TimeBasedCallback(timeToRun, funcToRun);
    timer.Start();
}

EDIT:
There are several functions that each writes to a DB every day in different hours. Can't afford task scheduler / process for each function. Quartz.NET seems a bit overkill for such a simple task.

Without knowing what you're trying to do, I'd say this is the wrong approach. I would create an executable and set it up as a scheduled task to run every 24 hours.

Not sure what OS you are using, but assuming it's windows its fairly easy http://windows.microsoft.com/en-au/windows7/schedule-a-task


Verify that a path doesn't contain . or .. or // and doesn't end with / This is actually a function from the first commit of the git source code, for the purpose stated in the title:

static int verify_path(char *path)
{
  char c;

  goto inside;
  for (;;) {
      if (!c)
          return 1;
      if (c == '/') {
inside:
          c = *path++;
          if (c != '/' && c != '.' && c != '\0')
              continue;
          return 0;
      }
      c = *path++;
  }
}

I was checking if there was an alternate way to implement this without the goto statement and ended up with this function:

static int verify_path(char *path)
{
    char c = '/';

    do {
        if (c == '/') {
            c = *path++;
            if (c == '/' || c == '.' || c == '\0')
                return 0;
        }
    } while ((c = *path++));

    return 1;
}

Do you see any problems with the new function? Is it missing anything from the original?

Let us please avoid discussing about goto.

Sample list of invalid path names:

/path/to/file
../path/to/file
path//to/file
path/../to/file
path/./to/file
path/to/.file
path/to/file/

Much better
I like your version much better than the original.  I don't see any cases you missed - it appears to work the same as the original function.  The only comment I have is that the path argument should be marked const.  But maybe you aren't allowed to change that.


JavaScript bottom-up tree transformer aimed for performance function transform(tree,fn){
    var root = tree,
        node = tree,
        child,
        parent,
        is_node,
        dir;
    root.dir = 0;
    while(true) {
        is_node = typeof(node)==="object";
        dir = is_node ? node.dir : 2;
        if (dir < 2)
            child = node[dir],
            node.dir++,
            child.parent = parent = node,
            child.dir = 0,
            node = child;
        else if ((changed = fn(node))!==undefined)
            changed.parent = parent,
            changed.dir = 0,
            node = changed;
        else
            if (!parent)
                return node;
            else
                parent[parent.dir-1] = node,
                node = parent,
                parent = node.parent;
    };
};

// TEST
var tree = [[1,2],[[3,4],[5,6]]];
console.log(
    JSON.stringify(transform(tree,function(a){
        if (a[0]===1) return [3,[5,5]];
        if (a[0]===5) return 77;
    })) === "[[3,77],[[3,4],77]]");

I need to make this function as fast as possible, no matter what. I'm aware this is far from the optimal implementation. How could I improve it? Asm.js, maybe?

Hmmm,

I don't think your code works,

var tree = [[1,2,[1,2,[5]]],[[3,4],[5,6]],[1,2,[1,2,[5]]],[[3,4],[5,6]]];

gives

[[3, 77], [[3, 4], 77], [1, 2, [1, 2, [5]]], [[3, 4], [5, 6]]]

I would have expected that you want that last [5,6] to be 77 in your fiddle.

Furthermore, as RoToRa mentioned

It is not clear in your code what you want to achieve
Comma separated statements to avoid curly braces are evil, python much?
Going through a tree pretty much asks for a recursive solution because you need to keep track of where you are

This function seems to do what you want thru recursion:

function transform( node, f )
{
  node = f(node) || node;
  var index = node.length;
  if( index )
  {
    while( index-- )
    {
      node[index] = transform( node[index] , f );
    }
  }
  return node;
}

Or, if you drop those curlies which you seem to like:

function transform( node, f )
{
  node = f(node) || node;
  var index = node.length;
  if( index )
    while( index-- )
      node[index] = transform( node[index] , f );
  return node;
}

I originally wanted to use [].map which would be cleaner, however you insisted speed.

Update: I checked jsperf for the fastest loop and it turns out there is a new loop variant that blows the other approaches out of the water: http://jsperf.com/loops/142

Using this approach the code would look like this:

function transform( node, f )
{
  node = f(node) || node;
  if( node.length )
    for (var i = 0, child; child = node[i]; i++)
      node[i] = transform( child , f );
  return node;
}


FactoryPattern without If-else construct Someone asked me if I can create factory pattern in java without using If-else construct. So I come with the following. Please provide your inputs if this seems a good example for using factories.

public enum EnumButtonFactory {

RADIO(RadioButton.class),
SUBMIT(SubmitButton.class),
NORMAL(NormalButton.class);

private Class<? extends Button> button;

EnumButtonFactory(Class<? extends Button> b) {
    this.button = b;
}

public Button get() {
    try {
        return button.newInstance();
    } catch (InstantiationException | IllegalAccessException e) {
        e.printStackTrace();
    }
    return null;
}
}

This is a good example for demonstrating that you can implement factory pattern without conditional statements. Of course, no piece of code is good for every imaginable requirement.

In real world cases factory pattern usually needed because some aspect of the object building is not trivial. One such case is when you do not have the concrete classes at compile time. e.g. when you are a library/framework developer and the user will have the concrete classes. In that case you would want to externalize the class names to the program configuration. This program requires that you have the SubmitButton.class et al at compile time, and is  dependent of those concrete classes.


Producer-Consumer Semaphore Implementation I have implemented a Producer-Consumer by Semaphore implementation.
I am looking forward to for your code review comments.
Thanks.
package ProducerConsumer.Semaphore;

import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Semaphore;

public class SemaphoreMain {

    public static void main(String args[]){

        List<Integer> arrayList = new LinkedList<>();
        int capacity = 50;

        Semaphore semaphoreProducer = new Semaphore(1);
        Semaphore semaphoreConsumer = new Semaphore(0);

        Producer producer = new Producer(arrayList, capacity, semaphoreProducer, semaphoreConsumer);
        Consumer consumer = new Consumer(arrayList, capacity, semaphoreProducer, semaphoreConsumer);

        Thread t1 = new Thread(producer);
        Thread t2 = new Thread(consumer);

        ExecutorService es = Executors.newFixedThreadPool(2);
        es.submit(t1);
        es.submit(t2);

        es.shutdown();

    }
}

Producer.java
package ProducerConsumer.Semaphore;

import java.util.List;
import java.util.concurrent.Semaphore;

public class Producer implements Runnable {

    List<Integer> linkedList;
    Semaphore semaphoreProducer;
    Semaphore semaphoreConsumer;
    int capacity;

    public Producer(List linkedList, int capacity, Semaphore semaphoreProducer, Semaphore semaphoreConsumer){
        this.linkedList = linkedList;
        this.capacity = capacity;
        this.semaphoreProducer = semaphoreProducer;
        this.semaphoreConsumer = semaphoreConsumer;
    }

    @Override
    public void run() {

        for (int i=0;i<50;i++){

                    try {
                        semaphoreProducer.acquire();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    linkedList.add(i);
                    System.out.println("Added" + i);
                    semaphoreConsumer.release();

        }

    }
}

Consumer.java
package ProducerConsumer.Semaphore;

import java.util.List;
import java.util.concurrent.Semaphore;

public class Consumer implements Runnable{

    List<Integer> linkedList;
    Semaphore semaphoreProducer;
    Semaphore semaphoreConsumer;

    public Consumer(List linkedList, int capacity, Semaphore semaphoreProducer, Semaphore semaphoreConsumer){
        this.linkedList = linkedList;
        this.semaphoreProducer = semaphoreProducer;
        this.semaphoreConsumer = semaphoreConsumer;
    }

    @Override
    public void run() {

        while(true){

                    try {
                        semaphoreConsumer.acquire();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    System.out.println("removed :" + linkedList.remove(0));
                    semaphoreProducer.release();

        }

    }
}

Capacity
You don't use capacity, yet you pass it into both your Producer/Consumer classes.  Superficially, it looks like you should be passing the capacity in when creating your Producer semaphore.  This would allow 'capacity' items to be put into your list.  At the moment, you're restricting it to only putting a single item into the list, then reading it back out again.
Thread safety
As it stands, your program works, however, it only works because you have a single consumer and producer.  The way you're using the semaphores means that either a consumer can be running, or the producer can be running, they can't both be running at the same time.  As a consequence, only one thread is working with the list.  If you start supporting more than one producer/consumer however you'll have the potential for having more threads using the collection concurrently... in which case you should consider alternate protection or using a concurrent collection, I'd suggest one of the Queues.
Naming
Some of your names are a little confusing / misleading...
List<Integer> arrayList = new LinkedList<>();

Creates a linked list that's called arrayList...
This is obviously subjective, but I'm also not a fan of semaphoreProducer... I can't help but expect it to be something that produces semaphores.  producerSemaphore works better for me, but as I said...very subjective.
Exit Strategy
Your Consumer thread doesn't have a way to exit, it just keeps on running.  Consider adding some way of notifying the consumer that it's time to shutdown so that the thread can be exited cleanly without you needing to force it to terminate.  This will become more important if your consumer starts doing some processing that takes a while that you don't want to exit in the middle of...


Filename interpreter for TV shows I have small class which interprets TV show filenames to extract the show title and episode number. Due to the nature of the filenames, I have chosen for a cleaning approach: I clean the filename first as much as possible and then extract the necessary data (instead of matching the data right away with regexp).

I'm kind of stuck on the following problem:

I have a cleanFilename() method and a getEpisodeNumber() method, which rely on each other to work but I want to be able to use them independently.

I would also like to note that the getEpisodeNumber() method requires a cleaned filename to work properly and that the cleanFilename() method requires the episode number to be removed for it to return the correct value.

NOTE: if anyone knows a better solution to extract the relevant data, please do let me know.

public function cleanFilename($filename)
{
    // run first string replace filter
    $step = str_replace($this->bad_filter, '', $filename);

    // replace underscore "_" with space " "
    $step = str_replace('_', '', $step);

    // run version filter
    $step = str_replace($this->version_filter, '', $step);

    // remove all not ascii characters
    $step = preg_replace('/[[:^print:]]/', '', $step);

    // remove everything between braces
    $step = preg_replace("/(\\[.*?\\])/is", '', $step);

    // remove everything between brackets
    $step = preg_replace("/(\\(.*\\))/is", '', $step);

    //remove episode number
    $step = str_ireplace($this->getEpisodeNumber($step, true), '', $step);

    //trim trailing dash
    $step = trim($step, ' -');

    return $step;

}

public function getEpisodeNumber($string, $filtered = false)
{
    if ($filtered === false) {
        $string = $this->cleanFilename($string);
    }

    // match integers
    $c = preg_match_all("/(\\d+)/is", $string, $matches);

    return sprintf('%02d', end($matches[0]));

}

These are some filenames:

[HorribleSubs] Shounen Hollywood - Holly Stage for 50 - 09 [480p].mkv.torrent
[DameDesuYo] Junketsu no Maria - 07 (1280x720 10bit AAC) [873A1506].mkv.torrent
[Cthuko] Shirobako - 20 [720p H264 AAC][6363865C].mkv.torrent
[Hatsuyuki]_Ansatsu_Kyoushitsu_OVA_[848x480][10bit][6BE57B34].mkv

This seems quite fine. I can only nitpick.

Here, either the comment is wrong or you have a bug,
as the command will remove underscore instead of replacing with space:

// replace underscore "_" with space " "
$step = str_replace('_', '', $step);

Many of the expressions have unnecessary elements:

The /i flag in expressions that don't include case sensitive pattern
The grouping within (...) in expressions that don't use the capture group
\\ instead of a simple \

For example these can be simplified:

$step = preg_replace("/(\\[.*?\\])/is", '', $step);
$step = preg_replace("/(\\(.*\\))/is", '', $step);
$c = preg_match_all("/(\\d+)/is", $string, $matches);

To this:

$step = preg_replace("/\[.*?\]/s", '', $step);
$step = preg_replace("/\(.*\)/s", '', $step);
$c = preg_match_all("/\d+/s", $string, $matches);

I would add one more operation to remove spaces in front of dots:

$step = preg_replace('/ +\./', '.', $step);


BFS implementation to find connected components taking too long This is in continuation to a question I asked here.  Given the total number of nodes (employees) and the adjacency list (friendship amongst employees), I need to find all the connected components.

public class Main {
    static HashMap<String, Set<String>> friendShips;

    public static void main(String[] args) throws IOException {
        BufferedReader  in= new BufferedReader(new InputStreamReader(System.in));

            String dataLine = in.readLine();
            String[] lineParts = dataLine.split(" ");
            int employeeCount = Integer.parseInt(lineParts[0]);
            int friendShipCount = Integer.parseInt(lineParts[1]);
            friendShips = new HashMap<String, Set<String>>();
            for (int i = 0; i < friendShipCount; i++) {
                String friendShipLine = in.readLine();
                String[] friendParts = friendShipLine.split(" ");
                mapFriends(friendParts[0], friendParts[1], friendShips);
                mapFriends(friendParts[1], friendParts[0], friendShips);
            }
            Set<String> employees = new HashSet<String>();
            for (int i = 1; i <= employeeCount; i++) {
                employees.add(Integer.toString(i));
            }
            Vector<Set<String>> friendBuckets = bucketizeEmployees(employees);
            System.out.println(friendBuckets.size());
    }

    public static void mapFriends(String friendA, String friendB, Map<String, Set<String>> friendsShipMap) {
        if (friendsShipMap.containsKey(friendA)) {
            friendsShipMap.get(friendA).add(friendB);
        } else {
            Set<String> friends = new HashSet<String>();
            friends.add(friendB);
            friendsShipMap.put(friendA, friends);
        }
    }

    public static Vector<Set<String>> bucketizeEmployees(Set<String> employees) {
        Vector<Set<String>> friendBuckets = new Vector<Set<String>>();
        while (!employees.isEmpty()) {
            String employee = getHeadElement(employees);
            Set<String> connectedEmployeesBucket = getConnectedFriends(employee);
            friendBuckets.add(connectedEmployeesBucket);
            employees.removeAll(connectedEmployeesBucket);
        }
        return friendBuckets;
    }

    private static Set<String> getConnectedFriends(String friend) {
        Set<String> connectedFriends = new HashSet<String>();
        connectedFriends.add(friend);
        Set<String> queuedFriends = new LinkedHashSet<String>();
        if (friendShips.get(friend) != null) {
            queuedFriends.addAll(friendShips.get(friend));
        }
        while (!queuedFriends.isEmpty()) {
            String poppedFriend = getHeadElement(queuedFriends);
            connectedFriends.add(poppedFriend);
            if (friendShips.containsKey(poppedFriend))
                for (String directFriend : friendShips.get(poppedFriend)) {
                    if (!connectedFriends.contains(directFriend) && !queuedFriends.contains(directFriend)) {
                        queuedFriends.add(directFriend);
                    }
                }
        }
        return connectedFriends;
    }

    private static String getHeadElement(Set<String> setFriends) {
        Iterator<String> iter = setFriends.iterator();
        String head = iter.next();
        iter.remove();
        return head;
    }
}

I have tested my code using the following script, the results of which I consume as sdtIn

#!/bin/bash
echo "100000 100000"
for i in {1..100000}
do
    r1=$(( $RANDOM % 100000 ))
    r2=$(( $RANDOM % 100000 ))
    echo "$r1 $r2"
   done

While I was able to verify (for trivial inputs) that my answer is correct, when I try with huge inputs as with the above script, I see that the run takes long (~20s).

Is there anything I can do better in my implementation ?

First of all, two things to read or search for: Cluster analysis (just a hint, I'm not an expert about that) and Linked: The New Science of Networks by Albert-László Barabási.

Barabási in his book shows that networks usually have some nodes which have a lot more connections than the others. The distribution is not the same in the real world as the sample shell script generates.
The code is quite good, I like your variable and method names and separated methods. I wonder why haven't anyone reviewed it yet.

Vector<Set<String>> friendBuckets = bucketizeEmployees(employees);

I'd use a simple List or ArrayList here. Vector is considered obsolete.
In the getConnectedFriends method the

Set<String> queuedFriends = new LinkedHashSet<String>();

could be a Queue. It has a poll method. As far as I tested it's faster than the currently used iterator-based remove.

public class Main {

Main isn't a good class name. Everyone can have a Main. What's it purpose? Try to find a more descriptive name.

static HashMap<String, Set<String>> friendShips;

HashMap<...> reference types should be simply Map<...>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

Should I always use the private access modifier for class fields?; Item 13 of Effective Java 2nd Edition: Minimize the accessibility of classes and members.

 static HashMap<String, Set<String>> friendShips;

Instead of Map<String, Set<String>> you could use Guava's Multimap (doc, javadoc) which was designed exactly for that. It would reduce the size of the mapFriends method:

public static void mapFriends(final String friendA, final String friendB,
        final Multimap<String, String> friendsShipMap) {
    friendsShipMap.put(friendA, friendB);
}

So, it could be removed.

public static Vector<Set<String>> bucketizeEmployees(Set<String> employees) {
    ...
}

This method calls getConnectedFriends(employee), which is the following:

private static Set<String> getConnectedFriends(String friend) {
    ...
}

It's confusing: what is the difference between an employee and friend? Are they the same?

if (friendShips.get(friend) != null) {

The following is the same:

if (friendShips.containsKey(friend)) {

A guard clause would be even better.

if (!friendShips.containsKey(friend)) {
    return connectedFriends;
}

if (!connectedFriends.contains(directFriend) && !queuedFriends.contains(directFriend)) {
    queuedFriends.add(directFriend);
}

The !queuedFriends.contains(directFriend) condition is unnecessary, it's a set which can't contain elements twice and adding an already added element to a LinkedHashSet doesn't modify anything. From the javadoc:

  Note that insertion order is not affected if an element is re-inserted into the set.

The following pattern occurs more than once in the code:

if (map.containsKey(key)) {
    String value = map.get(key);
    ...
}
...

It might be a microoptimization, but if you profile the code and the results shows it as a bottleneck the following structure is the same:

String value = map.get(key);
if (value != null) {
    ...
}
...

A few guard clause would help to make getConnectedFriends more flatten:

while (!queuedFriends.isEmpty()) {
    final String poppedFriend = getHeadElement(queuedFriends);
    connectedFriends.add(poppedFriend);
    if (!friendShips.containsKey(poppedFriend)) {
        continue;
    }
    for (final String directFriend: friendShips.get(poppedFriend)) {
        if (connectedFriends.contains(directFriend)) {
            continue;
        }
        queuedFriends.add(directFriend);
    }
}


Longest collatz sequence using dynamic programming I am trying to solve longest collatz sequence problem under 1000000 with the below code.  Can anyone suggest a faster way to approach this problem?  I was thinking of dynamic programming, but I'm having trouble in understanding it.

#include <stdio.h>

long long int col(long long int n)
{
    if(n==1)
    {
        return 1;
    }
    else
    {
        if(n%2==0)
            return (1+col(n/2));
        else
            return (1+col(3*n+1));
    }
}

int main()
{
    long long int i=0, c, max, k=1;
    max=1;
    for(i=1; i<1000000; i++)
    {
        c=col(i);
        if(c>max)
        {
            max=c;
            k=i;
        }
    }

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

Well, it appears you are having a bit of trouble getting things off-the-ground, so to speak. Let's start with the basics. Let's name your file collatz.c and after a quick glance, it looks like it should compile:

gcc -Wall -Wextra -o ctz collatz.c

Good, it compiled with no errors and no warnings. Now let's see if it will run:

output:

$ time ./ctz
525
837799

real    0m2.088s
user    0m2.082s
sys     0m0.004s

Also good, max is 525 and k is 837799 and it completed in less than 2.1 seconds. The logic is implemented in a single recursive function akin to that used by a non-math-lib power function, so no great speed improvements come to mind. As was pointed out, there are optimization that can help reduce the execution time. Let's try the suggested -Ofast -fwhole-program:

gcc -Wall -Wextra -o ctz collatz.c -Ofast -fwhole-program

output:

$ time ./ctz
525
837799

real    0m0.480s
user    0m0.474s
sys     0m0.003s

A 400+% improvement. That's better. So it looks like your work is done. Drop a comment or edit your question if you have more specifics in mind.


Frame limiting in an SDL game I am a relatively novice programmer, and I am taking steps into graphics programming. I am writing a quick pong clone and want to cap the FPS to save system resources. This is how I have implemented this (I've omitted other code for brevity)

#include <stdio.h>
#include <stdbool.h>
#include "SDL.h"
#include "Windows.h"
#define WIN32_LEAN_AND_MEAN
#define FPS 30

Uint32 starttime, endtime, deltatime;

int main()
{
 //initialize SDL and do other stuff
 //start game loop
 timeBeginPeriod(1);
 while (appisrunning)
 {
 starttime = GetTickCount();
    SDL_PollEvent(&event);
    if (event.type == SDL_QUIT)
    {
        SDL_DestroyRenderer(renderer);
        SDL_DestroyWindow(window);
        SDL_Quit();
        appisrunning = false;
        break;
    }
 // render stuff here
 endtime = GetTickCount();
    if (endtime > starttime)
    {
        deltatime = endtime - starttime;
    }
    else // handles wraparound
    {
        deltatime = ((2 ^ 32) - starttime) + endtime;
    }
    if (deltatime > (1000 / FPS)) {}
    else
    {
        Sleep((1000 / FPS) - deltatime);
    }
 }
 timeEndPeriod(1);
 return 0;
}

I use timeBeginPeriod(1) in an effort to give Sleep() a ~1ms resolution. According to the MSDN documentation, this function sets the timer resolution system wide and can result in higher CPU usage as the scheduler has to switch tasks more often (if I understand correctly).

Is this the right approach for this task? What feedback do you have for me?

Code does not handle "wrap-around" correctly.

2 ^ 32 is the exclusive-or of 32 and 2.  That has the value of 34.  Instead, as unsigned math is defined to wrap-around, just subtract.

//if (endtime > starttime) {
//    deltatime = endtime - starttime;
//} else {
//    deltatime = ((2 ^ 32) - starttime) + endtime;
//}

deltatime = endtime - starttime;

Missing objects

event, appisrunning, renderer, window are not obviously declared before use in SDL_PollEvent(&event);.  Perhaps an SDL thing?

Unnecessary use of global variables

Uint32 starttime, endtime, deltatime; belongs in main().  Or even better, within the loop.

Use an auto formatter

The below code hints at manual formatting.  Use an auto formatter and save time.

 // render stuff here
 endtime = GetTickCount();
    if (endtime > starttime)

An auto formatter would have the below and constant indent amount per level (rather than 2 and sometimes 4 spaces).

Example re-format

#include <stdio.h>
#include <stdbool.h>
#include "SDL.h"
#include "Windows.h"

#define WIN32_LEAN_AND_MEAN
#define FPS 30

int main(void) {
  timeBeginPeriod(1);
  while (appisrunning) {
    Uint32 starttime = GetTickCount();
    SDL_PollEvent(&event);

    if (event.type == SDL_QUIT) {
      SDL_DestroyRenderer(renderer);
      SDL_DestroyWindow(window);
      SDL_Quit();
      appisrunning = false;
      break;
    }

    Uint32 endtime = GetTickCount();
    Uint32 deltatime = endtime - starttime;

    if (deltatime > (1000 / FPS)) {
    } else {
      Sleep((1000 / FPS) - deltatime);
    }
  }
  timeEndPeriod(1);
  return 0;
}


Reusing thread and UdpClient for sending and receiving on same port The working and functional code below is a simplification of a real test application I've written. It acts as a client which asynchronously sends UDP data from a local endpoint and listens for an echo'ed response from a server implemented elsewhere.

The requirements are:

Send and listen on the same port number;
The port number must be able to change (simulates data coming in from various
sources)

The program does what I want, but I have doubts about its correctness. Specifically regarding the class members of listenThread and udpClient which are reused when the port number changes.  Are they implemented, closed and cleaned-up correctly.

using System;
using System.Net;
using System.Net.Sockets;
using System.Threading;

namespace cs_console
{
    class Tester
    {
        public void Send(string message, int localPort)
        {
            PrepareClient(localPort);
            SendBytes(System.Text.Encoding.UTF8.GetBytes(message));
        }

        private void PrepareClient(int localPort)
        {
            if (localEP == null || localEP.Port != localPort)
            {
                localEP = new IPEndPoint(IPAddress.Parse(localHost), localPort);
                listenThread = new Thread(StartListening);
                listenThread.Start();
            }
        }

        private void StartListening()
        {
            udpClient = new UdpClient();
            udpClient.ExclusiveAddressUse = false;
            udpClient.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
            udpClient.Client.Bind(localEP);

            var s = new UdpState(localEP, udpClient);
            currentAsyncResult = udpClient.BeginReceive(new AsyncCallback(ReceiveCallback), s);
        }

        private void ReceiveCallback(IAsyncResult ar)
        {
            if (ar == currentAsyncResult)
            {
                UdpClient c = (UdpClient)((UdpState)(ar.AsyncState)).c;
                IPEndPoint e = (IPEndPoint)((UdpState)(ar.AsyncState)).e;

                Byte[] buffer = c.EndReceive(ar, ref e);
                if (buffer.Length > 0)
                    System.Console.WriteLine(System.Text.Encoding.UTF8.GetString(buffer));

                UdpState s = new UdpState(e, c);
                currentAsyncResult = udpClient.BeginReceive(new AsyncCallback(ReceiveCallback), s);
            }
        }

        private void SendBytes(byte[] messageData)
        {
            var sender = new UdpClient();
            sender.ExclusiveAddressUse = false;
            sender.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
            sender.Client.Bind(localEP);
            sender.Send(messageData, messageData.Length, remoteEP);
            sender.Close();
        }

        private static string localHost = "10.27.21.109";
        private IPEndPoint localEP = null;
        private IPEndPoint remoteEP = new IPEndPoint(IPAddress.Parse(localHost), 11000);
        private Thread listenThread = null;
        private UdpClient udpClient = null;
        private IAsyncResult currentAsyncResult = null;

        private class UdpState : Object
        {
            public UdpState(IPEndPoint e, UdpClient c) { this.e = e; this.c = c; }
            public IPEndPoint e;
            public UdpClient c;
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            Tester tester = new Tester();
            int localPort = 2532;
            while (true)
            {
                ++localPort;
                var message = Console.ReadLine();
                tester.Send(message, localPort);
            }
        }
    }
}

This is as compact a working example as I could make.

I find a better naming convention for private fields on a class is _PascalCase or _camelCase. This makes it easy to see whether you are using a field or a local variable.
It is completely unnecessary to create a new Thread to call StartListening. The thread will terminate once udpClient.BeginReceive() has been called as this is async. You might as well call StartListening() directly in PrepareClient().
UdpClient is IDisposable and should therefor be disposed when not needed anymore. You do not clean up the existing UdpClient at all.

The answer to this question indicates that when you call Dispose or Close on a socket then the callback is invoked but will throw an ObjectDisposedException when calling EndReceive . Because you are storing the currentAsyncResult you can probably get around this by changing StartListening() to this:

private readonly object _clientLock = new object();

private void StartListening()
{
    lock (_clientLock)
    {
        if (udpClient != null)
        {
            currentAsyncResult = null;
            updClient.Dispose();
        }
        udpClient = new UdpClient();
        udpClient.ExclusiveAddressUse = false;
        udpClient.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
        udpClient.Client.Bind(localEP);

        var s = new UdpState(localEP, udpClient);
        currentAsyncResult = udpClient.BeginReceive(new AsyncCallback(ReceiveCallback), s);
    }
}

Plus you should lock the inner block of your receive callback. This should ensure that you don't kill the client in the middle of receiving.


Denormalizing an OrderedDict to CSV Cross post from stackoverflow, as suggested.

I have an ordereddict, and I need to store it in CSV file.
Input:

dic = collections.OrderedDict([('k1', 'v1'),
                               ('k2', 'v2'),
                               ('k3', [ [('k3a',21), ('k3b',22)],
                                        [('k3a',23), ('k3b',24)] ]),
                               ('k4', [1,2,3,4])])

mapping = {'k3': ['k3a', 'k3b']}

Desired output:

k1,k2,k3a,k3b,k4
v1,v2,21,22,1
v1,v2,23,24,2
v1,v2,,,3
v1,v2,,,4

That is, substitute the top level keys in the mapping dictionary with the sub-keys in the OrderedDict, and add blank fields when no data is available.

My working code so far is:

import csv
import collections

max_len = 0
result = []
for k,v in dic.items():
    if isinstance(v, list) and len(v) > max_len:
        max_len = len(v)

for i in range(max_len):
    tmp = collections.OrderedDict()
    for k, v in dic.items():
        if not isinstance(v, list):
            tmp.update({k:v})
        else:
            if k in mapping and i < len(v):
                if isinstance(v[i], list):
                    for tup in v[i]:
                        tmp.update({tup[0]:tup[1]})
            elif k in mapping and i >= len(v):
                for mk in mapping[k]:
                    tmp.update({mk:''})
            else:
                try:
                    tmp.update({k:v[i]})
                except IndexError:
                    tmp.update({k:''})
    result.append(tmp)

with open("out.csv", "w", newline='') as csvfile:
    writer = csv.DictWriter(csvfile, result[0].keys())
    writer.writeheader()
    writer.writerows(result)

Is there a cleaner/easier way to achieve the desired result?

tmp.update({k:v}) is an unusual way to perform tmp[k] = v
Take advantage of the max builtin:

max_len = max(len(v) for v in dic.values() if isinstance(v, list))

To make the code easier to follow, you could first transform the dictionary to give it a consistent structure like this

{'k1': ['v1', 'v1', 'v1', 'v1'],
 'k2': ['v2', 'v2', 'v2', 'v2'],
 'k3a': [21, 23],
 'k3b': [22, 24],
 'k4': [1, 2, 3, 4]}

using code like this

mapped = collections.OrderedDict()
for key, value in dic.items():
    if key in mapping:
        for sublist in value:
            for subkey, subvalue in sublist:
                mapped.setdefault(subkey, []).append(subvalue)
    elif isinstance(value, list):
        mapped[key] = value
    else:
        mapped[key] = max_len * [value]

Then you can take advantage of itertools.zip_longest to produce the output:

with open("out.csv", "w", newline='') as csvfile:
    writer = csv.writer(csvfile)
    writer.writerow(list(mapped.keys()))
    writer.writerows(itertools.zip_longest(*mapped.values(), fillvalue=''))


Password Generator with injected password model I'm getting started with Python and one of the beginner projects to learn the language was a password generator (one of the dozens already in CodeReview). I have a C# and JS background so I want to make sure I'm doing things the Python way.
Technically the include_punctuation and include_digits properties could be omitted and the exclusion list would be a custom concat of strings.digits+strings.punctuation.
The reasoning behind the PasswordModel is that any component that has an API of charset and length is a valid model. That would mean we could create a MemorablePasswordModel for passwords such as "apple truck butterfly postcard" with a clean and separate model.
from random import choice
from string import digits, ascii_letters, punctuation

def filter_exclusions(items, exclusions):
    result = []
    for i in items:
        if i not in exclusions:
            result.append(i)

    return result

class PasswordModel:
    def __init__(self, length=8, include_symbols=True, include_digits=True, exclusion_list=""):
        self.exclusion_list = list(exclusion_list)
        self.length = length
        self.include_symbols = include_symbols
        self.include_digits = include_digits

    def charset(self) -> str:
        chars = [ascii_letters]

        if self.include_symbols:
            chars.append(punctuation)
        if self.include_digits:
            chars.append(digits)

        return "".join(filter_exclusions("".join(chars), self.exclusion_list))

class PasswordGenerator:
    def __init__(self, password_model=PasswordModel()):
        self.model = password_model

    def generate(self) -> str:
        charset = self.model.charset()
        return "".join(choice(charset) for _ in range(self.model.length))

# Test it out
model = PasswordModel(length=24, exclusion_list="\\/@%")
gen = PasswordGenerator()

print(gen.generate())

Questions

Is there a cleaner way to "include" symbols and digits, I just appended them to a list.
Is there anything built-in to exclude one list from another?

Is there a cleaner way to "include" symbols and digits, I just appended them to a list.

Cast (rather than wrap) ascii_letters etc. to a set.

Is there anything built-in to exclude one list from another?

Yes - don't use lists; use sets and apply set subtraction.
Otherwise: if you perform a cursory scan of this site for Python-based password generators you will hear some of these same points, but -

Do not use random; use the secrets module to generate secure random numbers
PasswordGenerator being its own separate class does not make sense; it doesn't offer anything over generate so generate can be a free function or more likely a method on PasswordModel
charset would be better to return a set than a str
Maybe exclusion_list is being accepted as a string due to the valid concern that mutables (lists, etc.) should not be made parameter defaults. That's fine, though a type hint exclusion_list: Iterable[str] = "" should make it clear that other iterables are acceptable, such as a set or tuple.


Game of Life in Java I wrote John Conway's Game of Life in Java:

class GameOfLife {
    static int countSurrounding(int[][] board, int a, int b) {
        int count = 0;
        int[][] surrounding = {{a - 1, b - 1},
                               {a - 1, b    },
                               {a - 1, b + 1},
                               {a    , b - 1},
                               {a    , b + 1},
                               {a + 1, b - 1},
                               {a + 1, b    },
                               {a + 1, b + 1}};
        for (int i[]: surrounding) {
            try {
                if (board[i[0]][i[1]] == 1) {
                    count++;
                }
            }
            catch (ArrayIndexOutOfBoundsException e) {}
        }
        return count;
    }

    static void printBoard(int[][] board) {
        for (int[] i: board) {
            for (int j: i) {
                if (j == 1) {
                    System.out.print("#");
                }
                else {
                    System.out.print(".");
                }
            }
            System.out.println();
        }
        System.out.println();
    }

    public static void main(String[] args) {
        int[][] nextBoard = {{0, 0, 0, 0, 0, 0, 0, 0, 0},
                              {0, 0, 0, 1, 0, 0, 0, 0, 0},
                              {0, 1, 0, 1, 0, 0, 0, 0, 0},
                              {0, 0, 1, 1, 0, 0, 0, 0, 0},
                              {0, 0, 0, 0, 0, 0, 0, 0, 0},
                              {0, 0, 0, 0, 0, 0, 0, 0, 0},
                              {0, 0, 0, 0, 0, 0, 0, 0, 0},
                              {0, 0, 0, 0, 0, 0, 0, 0, 0},
                              {0, 0, 0, 0, 0, 0, 0, 0, 0}};
        int[][] board = new int[nextBoard.length][nextBoard[0].length];
        for (int gen = 0; gen < 25; gen++) {
            for (int i = 0; i < nextBoard.length; i++) {
                for (int j = 0; j < nextBoard[i].length; j++) {
                    board[i][j] = nextBoard[i][j];
                }
            }
            printBoard(board);
            for (int i = 0; i < board.length; i++) {
                for (int j = 0; j < board[i].length; j++) {
                    if (board[i][j] == 1 && !(countSurrounding(board, i, j) == 2 || countSurrounding(board, i, j) == 3)) {
                        nextBoard[i][j] = 0;
                    }
                    else if    (board[i][j] == 0 && countSurrounding(board, i, j) == 3) {
                        nextBoard[i][j] = 1;
                    }
                }
            }
        }
    }
}

How can I improve and optimize it?

A few suggestions:

Each cell in the Game of Life has exactly two states: dead or alive. Unless you plan to implement -1 as undead, I'd suggest you use booleans instead of integers to indicate the state.
The int i[] is a bit odd; i is normally used for indices, not for whole rows.
In countSurrounding you are using Exceptions to count the surrounding living cells. This is wasteful and unnecessary; we can determine the correct bounds ourself.

Given an index i in an array row, we can get the lower index min and the upper index max by

int minRow = i == 0 ? 0 : i - 1;
int maxRow = i == (row.length - 1) ? (row.length - 1) : i + 1;

The same thing for columns (j index in an array column):

int minCol = j == 0 ? 0 : j - 1;
int maxCol = j == (column.length - 1) ? (column.length - 1) : j + 1;

This itself assumes that i and j always are inside the bounds. We can then loop over the surrounding cells like

for (int row_idx = minRow; row_idx <= maxRow; row_idx++) {
   for (int idx = minCol; idx <= maxCol; idx++) {
     if (board[row_idx][idx] && !(row_idx == i && idx == j)) { // assumes board is implemented with booleans
        count++
     }
   }
}

While this is longer, this doesn't abuse exceptions for something totally non-exceptional.
The printBoard is fine, except for my first two criticisms. The if/else can be condensed to a ternary; I'd find this easier to read:

for (boolean row[] : board) {
   for (boolean cell : row) {
      System.out.print( cell ? "#" : "." );
   }
   System.out.println();
}

In main, you initialize nextBoard with the initial board, then copy it over into board. This is slightly useless. I'd propose to refactor your main into a method boolean[][] nextGeneration(boolean[][] board). Your main would then look like

int[][] initialBoard = ...;
public static void main(String[] args) {
   int[][] board = new int[nextBoard.length][nextBoard[0].length];
   // copy over the initial board
   for (int i = 0; i < initialBoard.length; i++) {
      for (int j = 0; j < initalBoard[i].length; j++) {
         board[i][j] = initialBoard[i][j];
      }
   }
   // loop over 25 generations
   printBoard(board);
   for (int gen = 0; gen < 25; gen++) {
      System.out.println("\nGeneration " + gen);
      board = nextGeneration(board);
      printBoard(board);
   }
}

Now something is becoming obvious: We have collected a few static methods that operate on our board array. This is a sign that we should create a seperate class:

Board
========
- board: boolean[][]
--------
- countSurrounding(i: int, j: int): int
+ toString(): String
+ nextGeneration(): Board

The class could be easily extended to have a static default board, or a constructor that creates a random board.
Each “Game of Life” can be described by a ruleset like 23/3. The digits before the slash determine when a living cell stays alive, the digits after the slash determine when a new cell is born. Generalizing nextGeneration to be able to play with all possible rules is a nice exercise.

private boolean nextCellState(int i, int j) {
   int count = countSurrounding(i, j);
   int rules[] = board[i][j] ? stayAlive : birthNew;
   for (int rule : rules) {
      if (rule == count) {
         return true;
      }
   }
   return false;
}

then

public Board nextGeneration() {
   boolean next[][] = new boolean[board.length][board[0].length];
   int rows = board.length;
   int cols = board.[0].length;
   for (int i = 0; i < rows; i++) {
      for (int j = 0; j < cols; j++) {
         next[i][j] = nextCellState(i, j);
      }
   }
   return new Board(next);
}


Recursively insert a node in an ordered Linked List I've written this and it works great. Is there any shorter or simpler way to achieve the same result without distinguish between cases?

Node* recursive_ordered_insert(Node* head, int val)
{
    //special case: first elem is null
    if (!head)
    {
        head = create_node(val);

        return head;
    }

    //special case 2: end of list
    if (!head->next)
    {
        head->next = create_node(val);

        return head->next;
    }

    //base case
    if (head->next && head->next->data > val)
    {
        Node* newNode = create_node(val);
        newNode->next = head->next;

        head->next = newNode;

        return newNode;
    }

    return recursive_ordered_insert(head->next, val);
}

The assumptions your code makes are... odd. Why do you have to use recursion. In reality, your code will overflow the stack for long lists, unless you ramp up the compiler optimization, and your compiler performs tail-call optimization on the recursion.

So, yes, you can do a recursive insert, and, assuming this is some sort of educational process, your implementation appears to be reasonable, with some caveats:

I don't like the "create if the list does not exist" logic in the first block. Using the head as an "output-parameter" is ugly. head should not be modified. The new head is returned anyway. The calling code should be able to tell if the head is null before it calls the insert method, and instead call a 'create' method of some sort. Additionally, that check is only ever needed once for the entire list, so it is a waste of time to check it for each level of recursion.
you cannot insert at the beginning of the list. If you are intending to insert the values {4, 3, 2, 1} and your first node starts as 4 at the 'head', well, you can't insert before that..... Your code requires that the first node is always the minimum value in order for the sort-order to be maintained.

Bottom line, is your code only works for limited use-cases.

I would much rather your code had a 'sentinel' head node, which was never null, and then your actual data follows from that head position. That would reduce your code, and make the insertion process simpler, and also fix both issues above. Note that the 'head' node would be kept as part of the class fields somewhere, not passed in as an argument to the (recursive) method... in fact, the method should not be recursive


Count groups of open cells around a cell on a grid Consider a 3x3 square surrounding a cell (marked @) on a grid:

0 1 1
1 @ 0
1 1 1

I need to find the number of groups of open cells (ones) surrounding the center cell. In the above example, there are two groups of open cells. All cells have the value 0, closed, or 1, open.
Here's what I have:

{
    // x and y coordinates for offsets corresponding
    // to the eight neighboring cells
    const xDir = [0, 1, 1, 1, 0,-1,-1,-1];
    const yDir = [1, 1, 0,-1,-1,-1, 0, 1];

    // generate a randomly open (1) or closed (0) 3 by 3 square of cells
    const generateSquare = _ => {
        const square = [];
        for (let x = 0; x < 3; x++) {
            square[x] = [];
            for (let y = 0; y < 3; y++) {
                square[x][y] = Number(Math.random() < 0.5);
            }
        }
        return square;
    };

    // count the number of contiguous groups of open (1) cells
    const countGroups = square => {
        let groups = 0;
        // loop clockwise through neighbors and count transitions from 0 to 1
        let prev = square[0][2];
        for (let i = 0; i < 8; i++) {
            let curr = square[1 + xDir[i]][1 + yDir[i]];
            groups += Number(!prev && curr);
            prev = curr;
        }
        // if there aren't any transitions, check if all neighbors are 1
        return !groups && prev ? 1 : groups;
    };

    const square = generateSquare();
    console.table(square);
    console.log(countGroups(square));
}

Is there anything I can improve?

You could try using ramdajs. I solved it as follows but it could be way better

// indexes of the order to iterate in array square
const cord = [6, 7, 8, 5, 2, 1, 0, 3, 6];
const binaryRand = ()=>  Number(Math.random() < 0.5);

//const square = [1, 1, 1, 1, 0, 1, 0, 1, 0];
const square = R.times(binaryRand,9)

const zeroToOne = (acc,curr,index)=>{
  if(index === 0) return 0;
  return acc+= Number(!square[index-1] && square[curr])
}

const countGroups = (square)=>
  ((R.sum(square) - cord[4]) < 8)?
     cord.reduce(zeroToOne,0) : 1;

countGroups(square);

here is the Ramdajs repel of the code.


3 sum algorithm - Trying Recently I started looking and refreshing about algorithms and decided to try to do the 3 sum algorithm. I did the code below:

public static int[] tripletSum(int[] A, int sum) {

    HashMap<Integer, Boolean> data = new HashMap<>();

    for (int i = 0; i < A.length; i++) {
        data.put(A[i], true);
    }

    int i = 0;
    int j = 1;

    while (i < A.length && j < A.length) {

        Integer a = A[i];

        Integer b = A[j];

        Integer delta = sum - (a + b);

        if (data.containsKey(delta) && a != delta && b != delta && (a + b + delta) == sum) {
            int[] result = {a, b, delta};
            return result;

        }
        i++;
        j++;
    }
    return null;
}

I'd like opinions about if it is good logic, time complexity, improvements. Any tips and comments will be great.

The piece of code below:

if (data.containsKey(remainder) && a != remainder && b != remainder && (a + b + remainder) == sum) {

is included to avoid to repeat values in a and b, and to guarantee the sum result as expected.

Thanks

There is no reason to use HashMap, you are not using the bools anywhere. So I think You could just use HashSet. Which you don't need to feed in foreach, instead simply pass the integers as constructor parameter.

var set = new HashSet(A);

You don't have to compare both i and j

i < A.length && j < A.length

if j is always one larger then i, just compare with j.

This condition is always true:

(a + b + delta) == sum

since that is the way it was obtained in the first place:

Integer delta = sum - (a + b);

This:

int[] result = {a, b, delta};
return result;

could be just

return {a, b, delta};

You are returning null where you claim to return int[], not sure if that's correct.

Anyway I think your implementation does not work as intended, because it only looks for sums of two consecutive and third anywhere. It fails for example on [5,1,2,1,8] looking for 15=5+2+8.