Animating Coloured Cogs The script is for rotating 5 different coloured gears (tandwiel) on my site.

I think the code can be improved and improved performance. 5x var paper isn't that smart for speed?

Getting the current code working faster in the normal state is the primary concern, but there are two smaller issues I know of that I would appreciate help with:

The code works most of the time, but when changing the browser size I have a problem with the rotating left and right: 1 gear will rotate the other direction, but starts to rotate very fast and finally slows down to normal speed. The longer you let it rotate at normal speed and then change browser, the longer the gear rotates too fast. It seems to me it rotates backwards all the missing angles til it will be normal again??
How to stop the JavaScript when the browser size is mobile. So it won't overload for mobile. The animation should start again when changing to bigger browser size.

Those two issues are not the primary concern.

var w = window.innerWidth;

if (w>=960) {window.direction = -1; }
else {window.direction = 1; }

window.onresize=function(){
var w = window.innerWidth;
if (w>=960) {window.direction = -1; }
else if (w<960 && w>=715){window.direction = 1; }
};

window.onload = function() {
var paper = new Raphael(document.getElementById('canvas_container'), 730, 750);

var paper = new Raphael(document.getElementById('tandwiel-1'), 240, 240);
var tandwiel1 = paper.image( "tandwiel-1.png",0,0,240,240);

var paper = new Raphael(document.getElementById('tandwiel-2'), 240, 240);
var tandwiel2 = paper.image( "tandwiel-2.png",0,0,240,240);

var paper = new Raphael(document.getElementById('tandwiel-3'), 240, 240);
var tandwiel3 = paper.image( "tandwiel-3.png",0,0,240,240);

var paper = new Raphael(document.getElementById('tandwiel-4'), 240, 240);
var tandwiel4 = paper.image( "tandwiel-4.png",0,0,240,240);

var paper = new Raphael(document.getElementById('tandwiel-5'), 240, 240);
var tandwiel5 = paper.image( "tandwiel-5.png",0,0,240,240);

var angle = 0;

setInterval( function() {
angle += 2;
tandwiel1.stop().animate( { "transform": "R" + angle }, 1000, "<>" );
}, 150 );

setInterval( function() {
angle += 2;
tandwiel2.stop().animate( { "transform": "R" + -angle }, 1000, "<>" );
}, 150 );

setInterval( function() {
angle += 2;
tandwiel3.stop().animate( { "transform": "R" + angle }, 1000, "<>" );
}, 150 );

setInterval( function() {
angle += 2;
tandwiel4.stop().animate( { "transform": "R" + -angle }, 1000, "<>" );
}, 150 );

setInterval( function() {
angle += 2;
tandwiel5.stop().animate( { "transform": "R" + direction*angle }, 1000, "<>" );
}, 150 );
}

Ok, now that I have a firm grasp on what you're doing, I think I have what you're looking for. The below still applies in terms of optimizing what you posted, but I think what you want is more along the lines of the following:

fiddle

var paper = Raphael("canvas", 500, 500);

var rectangles = {
   1 : paper.rect(100,100,90,40).attr({fill: "#44ddff"}) ,
   2 : paper.rect(200,100,90,40).attr({fill: "#4488ff"}) ,
   3 : paper.rect(100,200,90,40).attr({fill: "#44ddff"}) ,
   4 : paper.rect(200,200,90,40).attr({fill: "#4488ff"})
};

function repeat() {

    var runOnce = 0;

    for (var i in rectangles) {
        rectangles[i].animate(
            {transform:'...R' + (i%2===0?'':'-') + '360'} ,
            2000 ,
            function() {

                runOnce++;

                if (runOnce==4)
                    repeat();

            });
    }
};
repeat();

It still needs some tweaks for your cogs, but I think this is a more adept to the capabilities of the Raphael framework.

Kudos on tackling Raphael at an early stage of learning. It's quite advanced, but definitely fun.

The first recommendation I will make is that you shouldn't repeatedly call "setInterval". It is a large waste of resources. Also, you are not creating multiple papers, you are overwriting the existing variable. Honestly, between that and the reuse of angle I'm somewhat confused this code is working for you at all... I certainly cant get it to run on fiddle.

var w = window.innerWidth;

if (w>=960) {window.direction = -1; }
else {window.direction = 1; }

window.onresize=function(){
var w = window.innerWidth;
if (w>=960) {window.direction = -1; }
else if (w<960 && w>=715){window.direction = 1; }
};

window.onload = function() {

    var paper = new Raphael(document.getElementById('canvas_container'), 730, 750);

    var tandwiel1 = paper.image( "tandwiel-1.png",0,0,480,240);

    var tandwiel2 = paper.image( "tandwiel-2.png",0,0,720,240);

    var tandwiel3 = paper.image( "tandwiel-3.png",0,0,960,240);

    var tandwiel4 = paper.image( "tandwiel-4.png",0,0,1200,240);

    var tandwiel5 = paper.image( "tandwiel-5.png",0,0,1440,240);

    var angle = 0;

    setInterval( function() {

        angle += 2;

        tandwiel1.stop().animate( { "transform": "R" + angle }, 1000, "<>" );

        tandwiel2.stop().animate( { "transform": "R" + -angle }, 1000, "<>" );

        tandwiel3.stop().animate( { "transform": "R" + angle }, 1000, "<>" );

        tandwiel4.stop().animate( { "transform": "R" + -angle }, 1000, "<>" );

        tandwiel5.stop().animate( { "transform": "R" + direction*angle }, 1000, "<>" );

    }, 150 );
}

Next I would swap out the unreliable setInterval for a recursive setTimeout. But that is not necessary for this experiment. I use setInterval plenty when whipping up little tests such as this, it's a bit easier to set up.

Additionally, have you tried using a Raphael set to perform your animation with only one animate directive? It might speed it up a bit.

UPDATE:

Upon reconsideration, I realize why it would work. Even though you are overwriting the paper variable, the object has already been added to the DOM. This is not undone when the variable is modified/removed within JavaScript. Ether way, you would want to encapsulate all of your elements on one paper if they are related/displayed-near-each-other

UPDATE:

A further consideration. What I like to do is first create the Raphael objects so that I can access them by themselves

var myObjects = {

    tandwiel1 : paper.image( "tandwiel-1.png",0,0,480,240,

    tandwiel2 : paper.image( "tandwiel-2.png",0,0,720,240),

    tandwiel3 : paper.image( "tandwiel-3.png",0,0,960,240),

    tandwiel4 : paper.image( "tandwiel-4.png",0,0,1200,240),

    tandwiel5 : paper.image( "tandwiel-5.png",0,0,1440,240)
};

And then, if I need to animate them as a group, I create a set for easy manipulation.

var allTanwiels = paper.set(myObjects.tandwiel1 , myObjects.tandwiel2 , myObjects.tandwiel3 , myObjects.tandwiel4 , myObjects.tandwiel5)

allTanwiels.forEach(function(tanwiel) {
    tanwiel.stop().animate( { "transform": "R" + angle }, 1000, "<>" );
});


Ruby metaprogramming - declaring "properties" on a class I'm new to Ruby and trying to do a bit of metaprogramming. I want to declare some "properties" on a class, describing some field names and their access privileges:

class Resource
  # my_property(name, privilege => level)
  my_property :id, :read => :admin # :write => :none
  my_property :name, :read => :all, :write => :owner
  my_property :pin, :read => :owner, :write => :owner

  # ...
end

Where privileges are various symbols and levels are [:all, :owner, :admin], and higher level implies all lower levels.

Later, I want to retreive the information in this way:

res.readable_properties(:admin) # => [:id, :name, :pin]
res.writeable_properties(:admin) # => [:name, :pin]
res.writeable_properties(:all) # => []

Where method names are based on privilege names plus "able_properties" suffix.

Obviously, those declarations are on class level, so everything should be done only once when class is declared, and not on every instance creation.

I've tried to implement, but while it works fine, I suspect that my code is ugly and kludgy. Could someone please give me a hints on how to improve it? Here's what I came with:

require 'set'

class ResourceBase
  class << self
    def my_property(name, access)
      access_levels = %w[all owner admin].map {|s| s.to_sym}
      puts "#{name}: #{access}"

      @permissions ||= {}
      permit = {}
      access_levels.each do |level|
        access.each do |permission, min_level|
          @permissions[permission] ||= {}
          @permissions[permission][level] ||= []
          permit[permission] = true if level == min_level
          @permissions[permission][level].push(name) if permit[permission]
        end
      end

      @perm_methods ||= Set.new()
      @permissions.keys.each do |permission|
        next if @perm_methods.include? permission
        @perm_methods.add permission
        define_method "#{permission}able_properties".to_sym, do |level|
          self.class.permissions[permission][level]
        end
      end
    end

    def permissions
      @permissions
    end
  end
end

class Resource < ResourceBase
  # Levels range: all < owner < admin
  # Higher levels imply lower ones (i.e. admin always have owner access)
  my_property :id, :read => :admin #, :write => :none
  my_property :name, :read => :all, :write => :owner
  my_property :pin, :read => :owner, :write => :owner
end

resource = Resource.new
puts "Readable: " + resource.readable_properties(:admin).to_s
puts "Writeable: " + resource.writeable_properties(:admin).to_s

I seek for any suggestions which could be phrased as "Rather than [how OP did it] it's better to [how it should be done]."

Rather than access the possibly uninitialized @permissions instance variable inside my_property, use your wrapper method permissions and put the initialization logic in there. Also use Hash.new to save you the ||= statements. Also, the name list should be a Set, IMHO:

def permissions
  @permissions ||= Hash.new { |h,k| h[k] = Hash.new { |h,k| h[k] = Set.new }}
end

That way you can simply call

permissions[permission][level].add

without bothering if the intermediate hashes and sets have been initialized

Some other thoughts:

The second half of your my_property method is only about updating the access methods. This should be a separate method.
You don't use @perm_methods at all, you can remove it.
Use map(&:to_sym) (or even better, don't create the temporary string array in the first place, it's not shorter!).
Don't use to_sym on the first argument to define_method, it shouldn't be necessary.
The min_level part can be improved in style and efficiency. You have at least two options here to get all the affected levels:

Use array slicing: access_levels[access_levels.index(level)..-1]
Use drop_while: access_levels.drop_while { |level| level != min_level }

After cleaning this up a bit, I came around with the following (untested) code:

require 'set'

class ResourceBase
  class << self
    def my_property(name, access)
      access_levels = [ :all, :owner, :admin ]
      puts "#{name}: #{access}"

      access.each do |perm, min_level|
        access_levels.drop_while { |level| level != min_level }.each do |level|
          permissions[perm][level].add name
        end
      end
      update_permission_methods
    end

    def update_permission_methods
      permissions.each do |name, perm|
        define_method("#{name}able_properties") { |level| perm[level] }
      end
    end

    def permissions
      @permissions ||= Hash.new { |h,k| h[k] = Hash.new { |h,k| h[k] = Set.new }}
    end
  end
end

class Resource < ResourceBase
  # Levels range: all < owner < admin
  # Higher levels imply lower ones (i.e. admin always have owner access)
  my_property :id, :read => :admin #, :write => :none
  my_property :name, :read => :all, :write => :owner
  my_property :pin, :read => :owner, :write => :owner
end

resource = Resource.new
puts "Readable: " + resource.readable_properties(:admin).to_s
puts "Writeable: " + resource.writeable_properties(:admin).to_s


Loads two columns from excel - cleans up the data - and makes a new list of unmatched values I am comparing two columns of data (~20k rows) to find the items that do not match. Actually one of my files is an export from an Oracle database, but I have not gotten to the point of working with databases in python yet.

The code works as intended, but as I am new to this I am wondering if there is a more efficient way to do all of this.

First I read the columns into a list:

 import openpyxl
 wb = openpyxl.load_workbook('file.XLSX')
 ws = wb['dirtyfile']
 dirtysource = []
 cleansource = []
 for col in ws['A']:
     dirtysource.append(col.value)
 ws2 = wb['Database Export']
 for col in ws2['A']:
     cleansource.append(col.value)

The data I have extracted from the database is already cleaned. The other data set is in an inconsistent format:

dirtysource = [u'Tom Jones (01134)', u'Bob Smith (On Leave) (598711)', u'Jane Doe (00004)', u'100627 - Juan Carlos' ]

Next I clean the data up. I am slicing out the name, and just keeping the ID. The ID can be of a variable length. When it appears at the end of the string it is enclosed in parentheses. When it is at the beginning it is delimited by a '-'.

j = 0
for i in dirtysource:
    if i.endswith(')'):
        dirtysource[j] = str(i[i.rfind('(')+1:len(i)-1])
    else:
        dirtysource[j] = str(i[0:i.find('-')-1])
    j = j +1

The only thing I am doing to the clean list is to convert it from unicode:

j = 0
for i in cleansource:
    cleansource[j] = str(i)
    j = j + 1

Finally I compare the two lists to find the non-matching values:

delta = []
for i in dirtysource:
    if i in cleansource: continue
    else:
        delta.append(i)
print delta

delta1 = []
for i in cleansource:
    if i in dirtysource: continue
    else:
        delta1.append(i)
print delta1

Eventually I will be passing the two lists into some queries, but I have more to learn before I do that.

Your code is actually pretty good!

But there are some small things I would change:

Don't use ws and ws2, instead just index them in the for loop. E.g. for col in wb['dirtyfile']['A'].
Rather than initalizeing j and incrementing it each loop you can instead use enumerate.
It's part of Python so it's well known, and improves clarity in your code.
This can change your cleansource loop to:

for j, i in enumerate(cleansource):
    cleansource[j] = str(i)

Rather than checking if an item is in another list, if it is do nothing otherwise add to another list.
Just check if the value is not in the list. E.g.

for i in dirtysource:
    if i not in cleansource:
        delta.append(i)

You also tagged this performance. And so there is one way you can improve the performance of this program.
Since cleansource is a list, if i in cleansource is a performance killer.
Instead you can use a set, this can allow you to perform if i in cleansource faster, as it's optimised to do so.
However I'd still keep your original cleansource, as sets are an unique unorganised collection of data.
Say you have the list ['a', 'b', 'c', 'd', 'a', 'b'] and you change it to a set you could get {'b', 'd', 'c', 'a'} back.
Either way you can change your code to the following for a performance increase:

clean_set = set(cleansource)
for i in dirtysource:
    if i not in clean_set:
        delta.append(i)


Drawing a point on a chart Is there any way to make this work faster?

Here is my sample code in vb.net. This adds a point on a chart at mouse position but it is quite slow.

Private Sub Chart2_MouseMove(sender As Object, e As MouseEventArgs) Handles Chart2.MouseMove

Dim coord() As Double = GetAxisValuesFromMouse(e.X, e.Y)
Dim test As Series
Try
    Chart2.Series.RemoveAt(1)
Catch ex As Exception
End Try

Dim pt As New DataPoint
pt.XValue = coord(0)
pt.YValues(0) = coord(1)

test = New Series
Chart2.Series.Add(test)
Chart2.Series(test.Name).ChartType = SeriesChartType.Point
Chart2.Series(test.Name).Points.Add(pt)

End Sub

Function returns the coordinates of x and y axis at mouse position.

Private Function GetAxisValuesFromMouse(x As Integer, y As Integer) As Double()

Dim coord(1) As Double
Dim chartArea = Chart2.ChartAreas(0)
coord(0) = chartArea.AxisX.PixelPositionToValue(x)
coord(1) = chartArea.AxisY.PixelPositionToValue(y)
Return coord

End Function

Result:

Unless I'm mistaken, the MouseMove event fires everytime the mouse coordinates change - and when you're moving the mouse across the chart, that's a couple dozens (if not hundreds) of times in a very limited amount of time (<1 second!).

There are a couple things that stand out in your code:

Your indentation is off. You need to give the Tab button the lovin' it deserves!
An empty Catch block is bad. You're swallowing exceptions without ever knowing if anything has gone wrong. Don't do that!
GetAxisValuesFromMouse should be returning a Point, not an array of doubles. Returning a simple struct instead of an array can help you gain a couple cycles - my background is c# so the syntax might be a little off here:

Public Structure PointD ' named after System.Drawing.PointF which is using Float/Single.
    Public X As Double
    Public Y As Double
    Public Sub New(ByVal XValue As Double, ByVal YValue As Double)
        X = XValue
        Y = YValue
    End Sub
End Structure

Private Function GetAxisValuesFromMouse(x As Integer, y As Integer) As Double()

    return New PointD(chartArea.AxisX.PixelPositionToValue(x), _
                      chartArea.AxisY.PixelPositionToValue(y))

End Function

The chart object is abstracting a lot of plumbing, and I suspect this plumbing is where your bottleneck is. Your code removes a series, creates a new data point, adds a new series and adds the data point to the new series; the chart probably gets redrawn when you remove the series, and again when you add it, and once more when you add the point. It would be much more efficient to move the datapoint instead of recreating the series everytime the mouse moves by a pixel.

I suggest you only create the data point when you don't already have it (i.e. only once!); keep a reference to your data point, and simply change its coordinates per the mouse. If that alone doesn't do it, you'll need code that finds the data point and moves it. Since you're drawing the point on its own series, that shouldn't be too hard to achieve. The chart will only redraw itself when the coordinates of the point have changed, and when the mouse moves you'll only be fetching the mouse coordinates and assigning new values to the point's XY values. I'm sure this would be much faster.

And yeah, exceptions incur some performance hit, so if your code is swallowing exceptions that is another possible bottleneck, but if you're only moving a data point around, you shouldn't have to worry about that.


square the elements of a sorted list and give the output in sorted order Given a sorted list of integers, square the elements and give the
  output in sorted order.

  For example, given [-9, -2, 0, 2, 3], return [0, 4, 4, 9, 81].

My solution 1:

const square = el => el * el;
const sortAsc = (a, b) => a - b;
const sortSquare = list => list
  .map(square)
  .sort(sortAsc);

console.log(sortSquare([-9, -2, 0, 2, 3]));

My solution 2:

const sortSquare2 = list => {
  list.sort((a, b) => Math.abs(a) - Math.abs(b));
  for (let i = 0; i < list.length; i++) {
    list[i] = Math.pow(list[i], 2);
  }
  return list;
};

console.log(sortSquare2([-9, -2, 0, 2, 3]));

Is there a faster solution? I have the feeling you can do something with the fact that the list is sorted to begin with. But I can't think of a good one.

If you start at both ends and compare these values, you can step down towards the center, unshifting the values, squaring them and unshifting them to the resulting array.

You don't need to square the values until you add them to the array. You can compare to find the greater value by negating the left side.

One \$O(n)\$ solution is as follows

function sortSquares(arr) {
    const res = [];
    var start = 0; end = arr.length - 1, idx = end;
    while (start <= end) {
        res[idx--] = (-arr[start] > arr[end] ? arr[start++] : arr[end--]) ** 2;
    }
    return res;
}

// Test code
[
    [-9,-5,-0.5,0.6,1, 2, 3,8],
    [1, 2, 3,8],
    [-10, -8, -3, -1, -1, 1, 1, 1],
    [0.6,1, 2, 3,8],
    [-9,-6,-3,-2,-1],
    [-9,-6,-3,-2,-1,1,2,3,6,9],
].forEach(a => log(a, sortSquares(a)));

function log(data, data1) {
   data = "[" + data + "] -=>> [" + data1 + "]";
   info.appendChild(Object.assign(document.createElement("div"),{textContent: data}));
}


Is there a more concise and/or readable way to write the following method? Preface: I'm starting to learn C# and I don't want to port any of my bad habits from other languages, so I'm following convention wherever possible.

Using the default Visual Studio code formatting, this relatively simple function requires 14 lines of code.

public void add(int[] values, Func<int, int> transform = null)
{
    foreach (int v in values)
    {
        if (transform == null)
        {
            add(v);
        }
        else
        {
            add(transform(v));
        }
    }
}

My first thought was to use the ?? operator to do something like the following snippet, but apparently that's gibberish.

public void add(int[] values, Func<int, int> transform = null)
{
    foreach (int v in values)
    {
        add(transform(v) ?? v);
    }
}

Is there a more concise and/or readable way to write the following method?

There are at least two answers.
The short one:

public void add(int[] values, Func<int, int> transform = null)
{
    foreach (int v in values)
        add( (transform != null) ? transform(v) : v);
}

The efficient one:

public void add(int[] values, Func<int, int> transform = null)
{
    if( transform != null)
      foreach (int v in values)
        add( transform(v));
    else
      foreach (int v in values)
        add( v);
}

Not nice, but you wanted a short version.

Maybe someone has a LINQ at hand which does this even better.


Argparse implementation This is a follow up to Log probe requests of WiFi devices focussing on a specific element of the code.

How can I indent this code to make it look great and be well formatted?

parser = argparse.ArgumentParser(description='Collect WiFi probe requests')
parser.add_argument('-i', '--interface',
                    default=default_interface,
                    help='the interface used for monitoring')
parser.add_argument('--tshark-path',
                    default=distutils.spawn.find_executable("tshark"),
                    help='path to tshark binary')
parser.add_argument('--ifconfig-path',
                    default=distutils.spawn.find_executable("ifconfig"),
                    help='path to ifconfig')
parser.add_argument('--iwconfig-path',
                    default=distutils.spawn.find_executable("iwconfig"),
                    help='path to iwconfig')
parser.add_argument('-o', '--output',
                    default='-',
                    help='output file (path or - for stdout)')
parser.add_argument('-c', '--channel',
                    default='all',
                    help='channel/s to hop (i.e. 3 or 3,6,9 or 3-14 or all or 0 for current channel')
parser.add_argument('--verbose',
                    action='store_true',
                    help='verbose information')
parser.add_argument('-p', '--only-probes',
                    action='store_true',
                    help='only saves probe data spit by newline')
parser.add_argument('--delay',
                    default=5,
                    help='delay between channel change')
args = parser.parse_args()

Python has a style guide, which lays out guidelines for (among other things) code formatting. Your current code is compliant with the style guide, except that the help parameter for --channel means the line is too long.

You can easily avoid this by simply breaking the string across multiple lines:

parser.add_argument('-c', '--channel',
                    default='all',
                    help='channel/s to hop (i.e. 3 or 3,6,9 or 3-14 or all or 0 '
                    'for current channel')

Alternatively, you can shorten the message slightly and use one of the other recommended indent styles, which uses less horizontal space:

parser.add_argument(
    '-c', '--channel',
    default='all',
    help="channel/s to hop (e.g. '3', '3,6,9', '3-14', 'all', '0' (current channel)")


Simple login using ReactJS I have successfully implemented a simple login page using ReactJS by coding it all in JavaScript (a flexibility that I like) but I am still a beginner in React and would like to know whether this is efficient and modular and if anything can be improved.

"use strict";

function Login(email, password) {
  var LOGIN_FLAG = 1002
  $.post("/server/account/login.php", { email: email, password: password })
   .done(function(data) {
     var JSONData = tryParseJSON(data);
     if (!JSONData || JSONData["statusCode"] == LOGIN_FLAG) {
       ShowFailureAtDOM("container");
       return;
     }

     var JWTData = KJUR.jws.JWS.parse(JSONData["jwt"]);
     if (JWTData) {
       ShowSuccessAtDOM("container", JWTData.payloadObj.name);
     } else {
       ShowFailureAtDOM("container");
     }
   });
};

function ShowSuccessAtDOM(id, name) {
  ReactDOM.unmountComponentAtNode(document.getElementById(id));
  ReactDOM.render(
    <LoginSuccess name={name} />,
    document.getElementById(id)
  );
};

function ShowFailureAtDOM(id) {
  ReactDOM.unmountComponentAtNode(document.getElementById(id));
  ReactDOM.render(
    <LoginFail />,
    document.getElementById(id)
  );
};

var Header = React.createClass({
  render() {
    return (

Web App - Login

    )
  }
});

var LoginForm = React.createClass({
  ValidateLogin() {
    var email = this.refs.loginEmail.state.value;
    var password = this.refs.LoginPassword.state.value;
    Login(email, password);
  },
  render() {
    return (

        <LoginEmail ref="loginEmail"/>
        <LoginPassword ref="LoginPassword"/>

        <LoginSubmit ValidateLogin={this.ValidateLogin}/>

    )
  }
});

var LoginEmail = React.createClass({
  getInitialState() {
    return {value: null}
  },
  onChange(e) {
    this.setState({value: e.target.value});
  },
  render() {
    return (

Email:

        <input type="text" onChange={this.onChange}/>

    )
  }
});

var LoginPassword = React.createClass({
  getInitialState() {
    return {value: null}
  },
  onChange(e) {
    this.setState({value: e.target.value});
  },
  render() {
    return (

Password:

        <input type="password" onChange={this.onChange}/>

    )
  }
});

var LoginSubmit = React.createClass({
  onClick() {
    this.props.ValidateLogin();
  },
  render() {
    return (
      <button onClick={this.onClick}>Login</button>
    )
  }
});

var LoginSuccess = React.createClass({
  render() {
    return (

Login Success! Welcome Back, {this.props.name}

    )
  }
});

var LoginFail = React.createClass({
  render() {
    return (

Login FAIL...

    )
  }
});

ReactDOM.render(
  <LoginForm />,
  document.getElementById("container")
);

No, you are re-rendering the application manually.

function ShowSuccessAtDOM(id, name) {
  ReactDOM.unmountComponentAtNode(document.getElementById(id));
  ReactDOM.render(
    <LoginSuccess name={name} />,
    document.getElementById(id)
  );
};

function ShowFailureAtDOM(id) {
  ReactDOM.unmountComponentAtNode(document.getElementById(id));
  ReactDOM.render(
    <LoginFail />,
    document.getElementById(id)
  );
};

Better:

class LoginApp extends Component {
  constructor(props) {
    super(props)
  }

  render() {
    return (

        {this.props.errors.length > 0 && <LoginFailure {...props} />}
        {this.props.success &&
You are logged in!
}
        {!this.props.success && <LoginForm />}

    );
  }
}

ReactDOM.render(<LoginApp {...props} />,
                document.getElementById('react-root'))

Too pass the props into your application, there are many ways. One of the most popular is using some kind of flux architecture. There is a library called redux. Its really easy to use, you should take a look, then you will understand how easy it is, to implement the example above.


Functionally find the index of the minimum value in an array I come from a C#/JavaScript/TypeScript background and am learning F# by implementing fundamental algorithms. This is finding the minimum value in an array:

let findMinimumValueIndex (array: int[]): int =
    let rec find (index: int) (minimumValueIndex: int) =
        let findNext = find (index + 1)
        match index with
        | i when i >= array.Length -> minimumValueIndex // base case
        | i when array.[i] < array.[minimumValueIndex] -> findNext i
        | _ -> findNext minimumValueIndex
    find 0 0

It works. I like the partial application of arguments in the findNext function. I like the nested rec function.

Questions

How can we make this code more readable?
How can we make this code a better match of canonical F# style?
Should we be using if... then... else instead of match when?
Should we be using more explanatory variables?

Test

[<Fact>]
let ``findMinimum returns the index of the minimum value in an array`` () =
    let array = [|4; 5; 6; 7; 3; 2; 7; 4; 6; 8; 3|]
    let result = findMinimumValueIndex array
    Assert.StrictEqual(result, 5);

I think there's no need to use the match keyword here because you're not actually doing any pattern matching, only shadowing the name index with i. I've change it to use if/then/else.
Removing all of the type annotations makes the function more generic and more concise. I've done this below and used the function Array.length to allow the compiler to infer that array is an array. The input type has changed from int [] to 'a [] where 'a has comparison, making the function reusable for other array types. However, this also makes the function a lot slower for some reason so I've marked it as inline to get the re-usability benefits without the performance hit.
I think the findNext function isn't really necessary for readability, but that is probably down to personal preference. I noticed that it makes the function much much slower for a large array so I have removed it. Making it inline has a similar effect.

That leaves us with this:

let inline findMinimumValueIndex array =
    let rec find i minimumValueIndex =
        if i >= Array.length array then minimumValueIndex // base case
        elif array.[i] < array.[minimumValueIndex] then find (i + 1) i
        else find (i + 1) minimumValueIndex
    find 0 0

If I actually needed to do this in code, I would probably do this instead:

array |> Seq.indexed |> Seq.minBy snd |> fst

This is simple enough that it might not even be worth defining a function for, though it is quite slow compared to my refactoring of your function above.

However, for an empty input array this throws an exception, which isn't very functional. And your function returns 0 for an empty array, which can be ambiguous as it's also the result if the first item is smallest. So you could handle that case and return int option instead of int:

let inline minIndexSafe array =
    if array = [||] then None
    else array |> Seq.indexed |> Seq.minBy snd |> fst |> Some


Checking if text is NOT displayed I'm currently writing unit tests in Selenium (C#). I need to do two things in this particular test:

  Check if 2 errors (with html container) are displayed and check if another (given) error is not displayed.

The test looks like this:

bool errorWindowVisible = tmethods.IsElementVisible(driver, By.XPath("//div[@class='message error']"));
if (errorWindowVisible == false)
{
  Assert.Fail("No error notification displayed.");
}

string errorTextTemp = "Error 1 text";
bool productIssue = tmethods.IsElementVisible(driver, By.XPath("//*[contains(.,'" + errorTextTemp + "')]"));
errorTextTemp = "Error 2 text";
bool productIssueB = tmethods.IsElementVisible(driver, By.XPath("//*[contains(.,'" + errorTextTemp + "')]"));
if (productIssue == false || productIssueB == false)
{
  Assert.Fail("No information about error cause.");
}

// Checking if another one is not visible
errorTextTemp = "Extra error";
bool uploadedFileIssue = tmethods.IsElementNotVisible(driver, By.XPath("//*[contains(.,'" + errorTextTemp + "')]"));
if (uploadedFileIssue == false) // Should be set to true
{
  Assert.Fail("Extra error shown.");
}

I use two methods here:

public bool IsElementVisible(IWebDriver driver, By element)
{
    if (driver.FindElements(element).Count > 0)
    {
    if (driver.FindElement(element).Displayed)
        return true;
    else
        return false;
    }
    else
    {
    return false;
    }
}

public bool IsElementNotVisible(IWebDriver driver, By element)
{
    if (driver.FindElements(element).Count > 0)
    {
    if (driver.FindElement(element).Displayed)
        return false;
    else
        return false;
    }
    else
    {
    return true;
    }
}

So, all of that is working, but I am not sure about quality of that code.
On the other hand, I don't need anything over-fancy. It just should be good, clean and understandable.

Should I keep it intact or update some parts?

Your IsElementVisible method can be condensed to:
public bool IsElementVisible(IWebDriver driver, By element)
{
    return driver.FindElements(element).Count > 0
        && driver.FindElement(element).Displayed;
}

I guess the method IsElementNotVisible should return boolean negation of result of the IsElementVisible method:
public bool IsElementNotVisible(IWebDriver driver, By element)
{
    return !IsElementVisible(driver, element);
}

Boolean comparisons
Expressions x == true and just x are eqivalent.
Expressions x == false and just !x are also eqivalent.
For example, you don't need to write:
if (productIssue == false || productIssueB == false)

you could write instead:
if (!productIssue || !productIssueB)


Generating PNG barcodes from a CSV I wrote the following code to generate a series of barcodes given a CSV with a list of ISBNs in the first column and titles in the second, using the python-barcode library (which itself requires pillow to generate PNGs and JPGs). I am fairly new with Python and have had little formal training, and so would be interested in any and all feedback regarding functionality, formatting, and anything else I might be missing.

import os
import csv
import barcode
from barcode.writer import ImageWriter

def create_code_png(isbn,title):
    """Creates barcode, gives ISBN and title"""
    isbn13 = barcode.get_barcode_class('ean13')
    return isbn13(isbn, writer = ImageWriter())

def remove_forbidden(string_in):
    """Removes characters forbidden from windows file names from a string"""
    forbidden_chars = ":;<>\"\\/?*|."
    return "".join([char for char in string_in if char not in forbidden_chars])

def codes_from_csv(list_location,destination_folder):
    """Creates PNG Barcodes from a csv, placing them in a given folder"""
    with open(list_location, newline='') as csvfile:
        os.chdir(destination_folder)
        for row in csv.reader(csvfile, dialect='excel'):
            code = create_code_png(isbn:=row[0],title:=remove_forbidden(row[1]))
            code.save(isbn + " " + title)

if __name__ == "__main__":
    codes_from_csv("P:\\barcodes.csv","E:\\Barcodes")

Since I think this is worth more than just a comment on another answer, in your codes_from_csv you are (ab)using the new walrus operator. Assignment expressions are not the right thing to do here. What they do is assign local variables (outside of the function!), and then the values of those local variables (but not their names) are passed by position to the function.

A small example of how this can lead to unexpected results:

def f(a, b):
    return a // b

# Normal positional arguments, order matters
f(3, 1)
# 3
f(1, 3)
# 0

# Keyword arguments, order does not matter, only name
f(a=3, b=1)
# 3
f(b=1, a=3)
# 3

# Using the walrus operator, variables are assigned,
# but are passed as positional arguments.
# The names of the (external) variables do not matter to the function!
f(a:=3, b:=1)
# 3
f(b:=1, a:=3)
# 0

In other words the last example is equivalent to:

b = 1
a = 3
f(b, a)
# 0

This might be what you want, but since you are using the names of the arguments of the function, it probably isn't. In your code it just so happens to work because you use the same order as in the function definition. This might be a source of bugs at some future point, which could be very hard to find!


Selecting multiple filters based on the true/false conditions I wrote a method to select the filters (two or more) based on the true/false conditions. Here is the method I used for selecting the multiple filters:

    public T SetPropertyTypes<T>(bool residential, bool commercial) where T : IPage, new()
{

    // Residential Property Type Check logic
    if (residential && (ElementIsNotActive(() => FindElement(By.CssSelector(propertyTypeResidentialCss))).Invoke(Driver)))
        ClickButton(() => FindElement(By.CssSelector(propertyTypeResidentialCss)), "Residential");
    else if (ElementIsActive(() => FindElement(By.CssSelector(propertyTypeResidentialCss))).Invoke(Driver))
        ClickButton(() => FindElement(By.CssSelector(propertyTypeResidentialCss)), "Residential");

    // Commercial Property Type Check logic
    if (commercial && (ElementIsNotActive(() => FindElement(By.CssSelector(propertyTypeCommercialCss))).Invoke(Driver)))
        ClickButton(() => FindElement(By.CssSelector(propertyTypeCommercialCss)), "Commercial");
    else if (ElementIsActive(() => FindElement(By.CssSelector(propertyTypeCommercialCss))).Invoke(Driver))
        ClickButton(() => FindElement(By.CssSelector(propertyTypeCommercialCss)), "Commercial");
}

Later I found that the code is redundant and it could be better if I make it simpler and non-ambiguous. I'm new to C# and am not aware of many features. Is there any way to refactor this using any features of C#? I tried checking the variables using if-else-if but that's not working as intended.

Yes your code has a lot of redundancy.  It can be simplified down to this:

if (residential)
{
    ClickButton(() => FindElement(By.CssSelector(propertyTypeResidentialCss)), "Residential");
}
if (commercial)
{
    ClickButton(() => FindElement(By.CssSelector(propertyTypeCommercialCss)), "Commercial");
}


Remove extra spaces from a string without using Regex or any internal library methods The objective is to remove extra spaces from the below string:

  "   Today   it  is     going    to  rain.     "

so it looks like below

  "Today it is going to rain."

Please determine if the code will perform well and if there is a better solution (NO REGEX or LAMBDA).

static string CompactStringPerfect()
{
    string longString = "   Today   it  is     going    to  rain.     ";

    StringBuilder sb = new StringBuilder();
    char[] longChars = longString.ToCharArray();

    int spaceCount = 0;

    //Using standard method with no library help
    for (int i = 0; i < longChars.Length; i++)
    {
        //If space then keep a count and move on until nonspace char is found
        if (longChars[i] == 32)
        {
            spaceCount++;
            continue;
        }

        //If more than one space then append a single space
        if (spaceCount > 1 && sb.Length > 0)
            sb.Append(" ");

        //Append the non space character
        sb.Append(longChars[i]);

        //Reset the space count
        spaceCount = 1;
    }
    return sb.ToString();
}

A few things to point out.

char[] longChars = longString.ToCharArray();

strings in C# are array-like, and can be indexed directly. We don't need to convert it into a char[].

int spaceCount = 0;

We don't really need to keep track of how many spaces so much as whether the last character was a space. Therefore, this can become a boolean flag instead.

if (longChars[i] == 32)

This line has a magic number (32), and that makes it a little harder to understand. I'm guessing it's the Unicode number for the space character, but the fact that I have to guess makes it bad. Better would be: if(longChars[i] == ' '), but best would be: if(Char.IsWhiteSpace(longChars[i])). That catches all potential whitespace characters, and will be more robust. Also, it makes your intention much clearer.

if (spaceCount > 1 && sb.Length > 0)
    sb.Append(" ");

I always recommend using braces in light of goto fail.

//Reset the space count
spaceCount = 1;

This confuses me. You say you are "resetting" the space count, but the original value was 0, not 1, so I might (incorrectly) think this is a bug and try to fix it. The root of this issue lies in how you are tracking spaces.

So incorporating those suggestions, here's what I come up with:

public static string Despace(string longString) {
    StringBuilder sb = new StringBuilder();
    bool lastWasSpace = true; // True to eliminate leading spaces

    for(int i = 0; i < longString.Length; i++) {
        if(Char.IsWhiteSpace(longString[i]) && lastWasSpace) {
            continue;
        }

        lastWasSpace = Char.IsWhiteSpace(longString[i]);

        sb.Append(longString[i]);
    }

    // The last character might be a space
    if(Char.IsWhiteSpace(sb[sb.Length - 1])) {
        sb.Remove(sb.Length - 1, 1);
    }

    return sb.ToString();
}


Reversing input characters The given code reverses an array of input characters.

The additional while loop in reverse() function is unnecessary I guess.
Any other method or way to remove it?

I would prefer to keep reverse() as void.

#include <stdio.h>

#define MAXLEN 65535 /* maximum length of input line */

void reverse(char s[], int l);

/* reverses given input line */

void main()
{
    int i, c;
    char line[MAXLEN];

    i = 0;

    while(i < MAXLEN-1 && (c = getchar()) != EOF && c != '\n')  /* input characters in array */
    {
        line[i] = c;
        ++i;
    }

    reverse(line, i);
    printf("%s", line);
}

void reverse(char s[], int l)
{
    char rev[MAXLEN];
    int i, len;

    i = 0;
    len = l;    /* duplicate length of initial array */

    if(s[l] == '\0')
    {
        --l;
    }

    while(i < len)
    {
        rev[i] = s[l];  /* reversing */
        ++i;
        --l;
    }

    i = 0;

    while (i < len) /* what i think is unnecessary */
    {
        s[i] = rev[i];  /* storing back to initial array */
        ++i;
    }
}

Standard C requires that main() return int:

int main(void)

There's no need (for 20 years now) to write all local declarations before the code.  Introduce variables as you need them, so they can be initialised immediately:

{
    char line[MAXLEN];

    int i = 0;

There's no need for the extra storage rev in reverse().  We can reverse the string in place, by swapping first and last characters, then the second and second-last, and so on.

Use the Standard Library - it provides functions such as scanf(), getline(), strcpy() and strlen().  Don't write your own code to do these things - it makes your code much less clear, and causes readers to wonder what's being done that's different to the standard.


Add number of hours to 24 hour clock This function adds a number of hours to a 24 hour clock:

/**
 * @param {Integer} now The current hours
 * @param {Integer} add The number of hours to add
 */
function addHours(now, add){
    var h = (now + add) % 24;

    return h < 0 ? 24 + h : h;
};

addHours(2, 5) //7
addHours(23, 5) //4
addHours(23, -5) //18
addHours(72, 3) //3
addHours(0, 0) //0

The output should always be an integer in the range of 0 - 23.

I can't see any problem with it, but maybe you can. Please let me know if it can be improved or if there are any bugs.

Tiny question ;)

As @Rotora mentioned, now is not a fantastic parameter name, it conveys that I need to only pass who late it is 'now', add is a verb, also not brilliant as parameter naming goes
addHours( -500 , -100 ) returns -0, is that what you want ?
You have 4 lines of comment, 3 lines of code, 1 blank line, perhaps you have to much comment ? Try to not need comments by working harder on parameter names.
Since adding is commutative ( order does not matter ), I would consider naming the function sumHours and name the parameters hours1 and hours2

function sumHours(hours1, hours2){
    var sum = (hours1 + hours2) % 24;
    return sum < 0 ? 24 + sum : +sum;
};


Writing out names of files on local C drive to a text document This program writes out all the files in the current directory level to a text file, outputs an array of sub-directories at the current level, then repeats this process until there are no more sub-directories to search.  The code below take 50 seconds to run.

I was hoping I could get any pointers on the code in general (I'm very new to C#) and also any suggestions for improving the runtime.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;

namespace ComputerFileOutput
{
    class Program
    {
        // Prints a string array of files located in specified path
        static void filesArray(string path)
        {
                string[] array = Directory.GetFiles(path);
                addFilesToTextFile(array);
        }

        //Appends files to .txt document
        static void addFilesToTextFile(string[] fileArray)
        {
            //Append fileArray to current .txt document
            using (System.IO.StreamWriter file = new System.IO.StreamWriter(@"C:\ComputerFiles.txt", true))
            {
                for (int i = 0; i < fileArray.Length; ++i)
                {
                    file.WriteLine(fileArray[i]);
                }
            }
        }

        //Returns a string array of directories in a specified path
        static string[] getDirectories(string path)
        {
            return (Directory.GetDirectories(path));
        }

        static void Main(string[] args)
        {
            DateTime t1 = DateTime.Now;
            string[] startUp = { "------LIST OF ALL COMPUTER FILES------" };
            System.IO.File.WriteAllLines(@"C:\ComputerFiles.txt", startUp);

            //Get files in c directory; append to .txt file
            filesArray("C:\\");

            //Get array of primary directories in C drive
            string[] directoryArray = getDirectories("C:\\");

            //initialize subdirectory search
            bool moreDirectories = true;

            //Go through each directory
            while (moreDirectories == true)
            {
                 List<string> subDirectories = new List<string>();
                foreach(string directoryName in directoryArray)
                {
                    try
                    {
                        filesArray(directoryName);

                        subDirectories.AddRange(getDirectories(directoryName));
                    }
                    catch(Exception)
                    {
                        Console.WriteLine("Unathorized to access directory: {0}", directoryName);
                    }
                }

                directoryArray = subDirectories.ToArray();

                if (directoryArray.Count() == 0)
                {
                    moreDirectories = false;
                    break;
                }
            }

            // wait for input before exiting
            DateTime t2 = DateTime.Now;
            Console.WriteLine("Total seconds = {0}", (t2 - t1).TotalSeconds);
            Console.WriteLine("Press enter to finish");
            Console.ReadLine();

        }
    }
}

First of all, you shouldn't hardcode values like your C:\ or C:\ComputerFiles.txt. If you want to change this, you don't want to edit it at multiple places.

static void filesArray(string path)

The usual .Net naming convention for methods is PascalCase (you'll notice that Main has to follow this convention).

Also, filesArray is pretty bad name for this method. It doesn't tell you what the method does (that it prints somethings). On the other hand, it tells you about an implementation detail (it uses an array), which it shouldn't.

static void addFilesToTextFile(string[] fileArray)

Whenever you call this method, you open and then close a file. I'm not sure about this, but it could add a measurable overhead, considering that you're going to have huge amount of directories.

return (Directory.GetDirectories(path));

The outer parentheses are not necessary here, remove them.

static void Main(string[] args)

If you're not going to use command-line arguments, you can safely remove the args parameter.

DateTime t1 = DateTime.Now;

To measure runtime of your program accurately, you should usually use Stopwatch instead of DateTime. Though if the time is on the order of 50 s, you don't need millisecond accuracy, so DateTime is probably okay.

while (moreDirectories == true)

This whole loop could be simplified if you used Queue<T>. Something like:

//Get array of primary directories in C drive
var directories = new Queue<string>(getDirectories("C:\\"));

//Go through each directory
while (directories.Count > 0)
{
    string directoryName = directories.Dequeue();

    try
    {
        filesArray(directoryName);

        foreach (var subdirectory in getDirectories(directoryName))
            directories.Enqueue(subdirectory);
    }
    catch (Exception)
    {
        Console.WriteLine("Unathorized to access directory: {0}", directoryName);
    }
}

If you don't like that foreach, you could easily create an extension method for it, called something like EnqueueRange().

catch(Exception)

If you're expecting unauthorized access errors, then you should catch UnauthorizedAccessException, nothing else.


Improvements on tree index class I developed a QuickIndex class which serves to index arrays of stringafiable objects using a tree structure. The purpose is to then primarily to allow for fast index or include? method calls directed to the quick_index object as a proxy for the original array, as the index tree can be traversed much more efficiently than a linear array.

There is a gist containing the whole class, but the main functionality comes from the two methods included below.

class QuickIndex
  def initialize ary, stop_char = "!"
    @stop_char = stop_char
    @size = ary.size
    @index = Hash.new
    ary.each_with_index do |num, i|
      num = num.to_s.split("") << @stop_char
      index = @index
      until num.empty?
        n = num.shift
        index = index[n] ||= (n == @stop_char ? [] : {})
      end
      index << i
    end
  end

  def index item
    index = @index
    (item.to_s.split("") << @stop_char).each { |n| next unless index = (index[n] or nil rescue nil) }
    index
  end
end

The stop_char is a single character which serves to indicate the end of a string in the index.

The class is specifically intended for locating specific values in very large (NArray) arrays of ints or floats, but it's nice for it to work with other objects too.

It works reasonably well, but I'd like to know of any optimisations or alternative strategies to this problem which would make the class quicker at either building or querying the index and/or reduce the memory footprint. Or if there's some standard library alternative I've overlooked...

You can build it faster and use it faster if you use a flat hash instead of cascading hashes.

Here's a simple implementation using a "flat" hash:

class QuickerIndex

  def initialize(array)
    @index = {}
    array.each_with_index do |item, i|
      (@index[item.to_s] ||= []) << i
    end
  end

  def index(item)
    @index[item.to_s]
  end

end

and a benchmark to compare the two:

require 'benchmark'
floats = 10_000.times.map{rand}
Benchmark.benchmark('', 20) do |x|
  index = nil
  x.report('cascading create:') {index = QuickIndex.new(floats)}
  x.report('cascading access:') do
    floats.each do |float|
      index.index(float)
    end
  end
  x.report('flat create:') {index = QuickerIndex.new(floats)}
  x.report('flat access:')  do
    floats.each do |float|
      index.index(float)
    end
  end
end

CPU

No duplicate items

Without duplicate items, the CPU time needed to create and access various sizes of indices were:

                    ------CREATE--------     -----ACCESS------
NUMBER OF FLOATS    CASCADING       FLAT     CASCADING    FLAT
    10,000             0.27         0.05          0.20    0.03
   100,000             3.98         0.45          2.42    0.43
 1,000,000           140.75        14.13         69.72    4.84

With no duplicate items, flat hash scales a little better than cascading hash in create, but much better in access.  Flat hash is faster for create and access.

Duplicate items

Now, what about repeated items?  You indicate that there is some likelihood that two items will share the same key (that is, have the same #to_s).  Let's assume that each number is duplicated 10 times.  That is, in a list of 10,000 floats, there are really 1,000 unique floats; in a list of 100,000 floats, there are 10,000 unique floats:

                    ------CREATE--------     -----ACCESS------
NUMBER OF FLOATS    CASCADING       FLAT     CASCADING    FLAT
    10,000              0.23        0.02       0.20       0.02
   100,000              2.20        0.26       2.12       0.21
 1,000,000             27.70        3.56      26.29       2.62
10,000,000            364.14       25.16     341.66      23.44

Both flat and cascading hash perform better with duplicate items than without.  Flat hash still scales better, and is always faster.

Memory

Unless compiled with the right switch, Ruby lacks an effective way to tell how much memory is in use by its objects.  As a proxy, we can use "ps", which reports the total process size in kilobytes.  It's an imperfect proxy, but it's the best we have.

Here are the numbers for various numbers of floats.  The number of unique floats was always kept at 1/10th the total number of floats.  The memory size reported is the amount that memory usage went up after creating the instance of the index class:

NUMBER OF FLOATS    CASCADING        FLAT
    10,000             2,048k         52k
   100,000            32,952k      1,664k
 1,000,000           232,868k     28,596k
10,000,000         2,232,568K    229,512k


Print matrix diagonals method I need to print (or store) all diagonals of a matrix. This includes the main diagonal ( { (i,i) | i = 1, 2, ... min(m,n) } ). Then all super and sub diagonals.

For example:

 1  2  3  4
 5  6  7  8
 9 10 11 12
13 14 15 16

The diagonals should be (order doesn't matter):

[
    [1, 6, 11, 16],
    [2, 7, 12],
    [5, 10, 15],
    [3, 8],
    [9, 14],
    [4],
    [13]
]

I believe I have a decent solution, but if there's a simpler / more efficient way to do this let me know. Input is assumed to be a list of lists as a NumPy array already has some methods to do this.

def print_diags(Matrix):
    m, n = len(Matrix), len(Matrix[0])
    for offset in range(min(m,n)):
        diag_upper = [row[i + offset]
                      for i, row in enumerate(Matrix)
                      if 0 <= i + offset < n]
        if offset != 0:
            diag_lower = [row[i - offset]
                      for i, row in enumerate(matrix)
                      if 0 <= i - offset < m]
            print(diag_upper, diag_lower)
        else:
            print(diag_upper)

Here's a generator in a generator:

m = [[1, 2, 3, 4], [5, 6, 7, 8], [9, 10, 11, 12], [13, 14, 15, 16]]

def diags(mat):
    width, height = len(mat[0]), len(mat)
    def diag(sx, sy):
        for x, y in zip(range(sx, height), range(sy, width)):
            yield mat[x][y]
    for sx in range(height):
        yield list(diag(sx, 0))
    for sy in range(1, width):
        yield list(diag(0, sy))

Usage:

>>> list(diags(m))
[[1, 6, 11, 16], [5, 10, 15], [9, 14], [13], [2, 7, 12], [3, 8], [4]]

diag starts from a coordinate and walks down the diagonal, yielding elements from it. It doesn't raise an IndexError, because zip exits after the shortest iterable is exhausted.

The last four lines are a bit inelegant, but I don't know a better way.


Find the Next Greatest Element Given an array, print the Next Greater Element (NGE) for every element. The Next Greater Element for an element x is the first greater element on the right side of x in array. Elements for which no greater element exist, consider next greater element as -1.

Examples:

For any array, rightmost element always has next greater element as -1.
For an array which is sorted in decreasing order, all elements have next greater element as -1.
For the input array [4, 5, 2, 25}, the next greater elements for each element are as follows:

    Element       NGE
       4      -->   5
       5      -->   25
       2      -->   25
       25     -->   -1

For the input array [13, 7, 6, 12}, the next greater elements for each element are as follows.

    Element          NGE
      13      -->     -1
      7       -->     12
      6       -->     12
      12      -->     -1

final class DataSet {

    private final int x;
    private final int nextX;

    public DataSet(int x, int nextX) {
        this.x  = x;
        this.nextX = nextX;
    }

    public int getX() {
        return x;
    }

    public int getNextX() {
        return nextX;
    }

    @Override
    public String toString() {
        return "x: " + x + " nextX: " + nextX;
    }

    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + nextX;
        result = prime * result + x;
        return result;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        DataSet other = (DataSet) obj;
        if (nextX != other.nextX)
            return false;
        if (x != other.x)
            return false;
        return true;
    }
}

public final class NextGreatestElement {

    private NextGreatestElement() {}

    /**
     * Computes list of dataset, where dataset is a tuple of
     * (currentInt, nextGreatestelement)
     *
     * @param a     the array
     * @return      list of dataset
     */
    public static List<DataSet> getNextGreatestElement(int[] a) {

        final List<DataSet> dataSetList = new ArrayList<DataSet>();
        final Stack<Integer> stack = new Stack<Integer>();

        for (int i = 0; i < a.length; i++) {

            while (!stack.isEmpty() && stack.peek() < a[i]) {
                dataSetList.add(new DataSet(stack.pop(), a[i]));
            }

            stack.push(a[i]);
        }

        while (!stack.isEmpty()) {
            dataSetList.add(new DataSet(stack.pop(), -1));
        }

        return dataSetList;
    }
}

public class NextGreatestElementTest {

    @Test
    public void test1() {
        int[] a1 = {2, 3, 4, 5};
        List<DataSet> ds1 = new ArrayList<DataSet>();
        ds1.add(new DataSet(2, 3));
        ds1.add(new DataSet(3, 4));
        ds1.add(new DataSet(4, 5));
        ds1.add(new DataSet(5, -1));
        assertEquals(ds1,  NextGreatestElement.getNextGreatestElement(a1));
    }

    @Test
    public void test2() {
        int[] a2 = {3, 2, 1};
        List<DataSet> ds2 = new ArrayList<DataSet>();
        ds2.add(new DataSet(1, -1));
        ds2.add(new DataSet(2, -1));
        ds2.add(new DataSet(3, -1));
        assertEquals(ds2, NextGreatestElement.getNextGreatestElement(a2));
    }

}

You have implemented a 'huge' data structure to solve the problem. In this case though, the solution is to use a relatively simple case of memoization. If you work backward from the end of the array, you can use what you have already learned for previous values.....

... in essence, you can use the subsequent NGE's to bootstrap your way to calculate the current cell's NGE.

private static int[] computeNGE(int[] tocompute) {
    if (tocompute == null) {
        throw new NullPointerException();
    }
    if (tocompute.length == 0) {
        return new int[0];
    }
    if (tocompute.length == 1) {
        return new int[]{-1};
    }
    int[] indices = new int[tocompute.length];
    int[] nge = new int[tocompute.length];
    nge[tocompute.length - 1] = -1;
    indices[tocompute.length - 1] = -1;
    for (int i = tocompute.length - 2; i >= 0; i--) {
        int j = i + 1;
        while (j != -1 && tocompute[j] <= tocompute[i]) {
            j = indices[j];
        }
        indices[i] = j;
        nge[i] = j == -1 ? -1 : tocompute[j];
    }
    return nge;
}

The above solution will have close to an \$O(n)\$ time complexity, because, the worst case is a 2-times iteration through the data.


Print all possible subsets of an array I solved the problem to print all possible subsets of an array. I just want to know a better approach or anything different I could have done.

Question: Print all possible subsets of an array.

My approach: I know that there are \$2^N\$ (no. of elements in the set) subsets and each subset can be represented with the help of \$N\$-bit binary number. As a part of an assignment I have already made a decimal to binary and to reverse(prints no. in reverse digit by digit) a number program.So I combined the two to solve this question.

Input: {1, 2, 3}

Output: 3, 2, 32, 1, 31, 21, 321

My Code:

private static void subset(int[] arr) {
    int size = arr.length;
    int binaryLimit = (int) Math.pow(2, size) - 1;
    for (int i = 0; i <= binaryLimit; i++) {       //generate decimal no.
        //generate corresponding binary no & counter(how many bit binary no. is)
        int N = i;
        int counter = 0;
        int power = 0, binary = 0;
        while (N != 0) {
            int rem = N % 2;
            rem = rem * ((int) Math.pow(10, power));
            binary = binary + rem;
            N = N / 2;
            power++;
            counter++;
        }
        // loop to traverse array
        for (int j = arr.length - 1; j >= arr.length - counter; j--) {
            // if binary no is greater then one digit, compare each digit separately.
            if (counter > 1) {
                int k = binary;
                while (k != 0) {
                    int rem = k % 10;
                    if (rem == 1) {     //if binary digit == 1, print corresponding no. from array
                        System.out.print(arr[j]);
                    }
                    j--;
                    k = k / 10;
                }
                System.out.println();
            } else {
                if (binary == 1) {
                    System.out.println(arr[j]);
                    j--;
                }
            }
        }
    }
}

Simplify

This could be written a lot simpler.
For each number until binaryLimit,
you could iterate over the bits using bit shifting,
and moving the current index in the array,
printing values when the bit is set.

private static void printAllSubsets(int... arr) {
    int size = arr.length;
    int binaryLimit = (1 << size) - 1;

    for (int i = 1; i <= binaryLimit; i++) {
        int index = size - 1;
        int num = i;
        while (num > 0) {
            if ((num & 1) == 1) {
                System.out.print(arr[index]);
            }
            index--;
            num >>= 1;
        }
        System.out.println();
    }
}

Calculating powers of 2

Note that Math.pow takes and returns double. Converting between types is ugly, and what's worse, floating point math is fraught with problems in general. Fortunately, for calculating powers of 2, a clean and blazingly fast alternative is to use bit shifting, as demonstrated in the code above.

Naming

subset is a poor name for a void method that prints subsets of elements of an array.

Usability

As there is no punctuation between elements as they are printed,
this method will not be very useful for larger arrays,
or arrays where values have more than one digit.

Java conventions

Capitalized variable names are normally used for constants,
never for local variables.

Method decomposition

Your implementation is very long,
and could have been split to smaller pieces that are easier to read and test individually.


Best Practices Generics in Typescript I'm wondering and actually playing in typescript playground with generics functions. The question is, is the code below readable? If not what kind of approach I could use to make it more typescript way and more readable?
Working example here
I implemented some basic functions like foldl, map and filter to experiment generics, maybe the implementation don't satisfies all requirements in the sense of functionality.
type FoldlFunction<T> = (a: T, b: T) => T;

function Foldl<T>(f: FoldlFunction<T>, base: T, arr: T[]): T {
    const [head, ...rest] = arr;
    return !head ? base : Foldl(f, f(base, head), rest);
}

console.log(Foldl((a: number, b: number) => (a + b), 0, [1,2,3,4,12,14,15,16]));

type MMapFunction<T,R> = (a: T) => R;

function MMap<T,R>(f: MMapFunction<T,R>, arr: T[]): R[] {
    const [head, ...rest] = arr;
    return !head ? [] : [f(head), ...MMap(f, rest)];
}

console.log(MMap((x: number) => (x * x), [1,2,3,4,12,14,15,16]));

type FilterFunction<T> = (a: T) => boolean;

function Filter<T>(f: FilterFunction<T>, arr: T[]): T[] {
    const [head, ...rest] = arr;
    return !head ? [] :
        (f(head) ? [head, ...Filter(f, rest)] : [...Filter(f, rest)])
}

console.log(Filter((x: number) => (x % 2 === 0), [1,2,3,4,12,14,15,16]));

Your code looks quite reasonable, though there are a few tweaks you could consider.
Function or Callback? Rather than names like FoldlFunction and MMapFunction, since what those types contain are the types for the callback run for each element of the array, consider calling them FoldlCallback etc instead. It's a bit more precise and will make more intuitive sense at a glance. (One might think at a glance that FoldlFunction might refer to the type of the whole Foldl function, which is incorrect.)
camelCase is the near-universal convention for plain non-class functions and variables in JavaScript. PascalCase is generally used for class constructors and namespaces, which aren't being used here, so camelCase is probably more appropriate. Eg Foldl -> foldl (or to foldL).
DRYer types You only need to specify the type of a parameter when TypeScript can't infer it automatically. For example, with foldl, since the base value is T, and the array is T[], the callback - the f: FoldlFunction<T> - will be known to be of the form (a: T, b: T) => T. There's no need to additionally specify the arguments' types when calling foldl. That is, this:
foldl((a: number, b: number) => (a + b), 0, [1,2,3,4,12,14,15,16])

can be
foldl((a, b) => (a + b), 0, [1,2,3,4,12,14,15,16])

It's a good idea to let TypeScript automatically infer the types of variables and parameters whenever possible - less code to read makes comprehension easier. You can do the same thing for MMap and Filter.
Outside definition? I'm not entirely convinced of defining the type of the function/callback outside - it's one more type parameter that needs to be cognitively pieced together. Consider defining the callback type in the parameter list:
function foldl<T>(
    f: (a: T, b: T) => T,
    base: T,
    arr: T[]
): T {
    const [head, ...rest] = arr;
    return !head ? base : foldl(f, f(base, head), rest);
}

console.log(foldl((a, b) => (a + b), 0, [1,2,3,4,12,14,15,16]));

I consider that significantly more readable. You can do the same thing for MMap and Filter.
Falsey bug You use return !head ? base : foldl.... This will not run if head is falsey. For example, foldl((a, b) => (a + b), 0, [1,2,3,4,12,14,15,16]) results in 67, but foldl((a, b) => (a + b), 0, [0, 1,2,3,4,12,14,15,16]) results in 0, which probably isn't desirable. Check the array's length instead:
function foldl<T>(
    f: (a: T, b: T) => T,
    base: T,
    arr: T[]
): T {
    if (!arr.length) {
        return base;
    }
    const [head, ...rest] = arr;
    return foldl(f, f(base, head), rest);
}

This applies to MMap and Filter as well.
DRYer filter To fix the falsey bug and make Filter a bit cleaner, you can replace:
const [head, ...rest] = arr;
return !head ? [] :
    (f(head) ? [head, ...Filter(f, rest)] : [...Filter(f, rest)])

with
if (!arr.length) {
  return [];
}
const includeThis = f(head); // call this here to preserve callback execution order
const restFiltered = Filter(f, rest);
return includeThis ? [head, ...restFiltered] : restFiltered;


All permutations of a sum I'm attempting to implement an algorithm that finds all subsets that sum to a number n. Here is my attempt so far:

function combos(n, combo, result){
  combo = combo || [];
  result = result || [];

  if(sum(combo) === n) {

    result.push(combo);
    return;
  }

  if(sum(combo) > n) {
    return;
  }

  for(var i = 1; i <=n; i++) {
    var newArr = combo.concat([i]);
    combos(n, newArr, result);
  }

  return result;
}

function sum(array) {
  if(!Array.isArray(array) || array.length === 0){
    return;
  }

  return array.reduce((element, sum) =>{
    return element + sum;
  }, 0);
}

This gives me all combinations. However, there are some redundant solutions. For example.

n = 3 gives [ [ 1, 1, 1 ], [ 1, 2 ], [ 2, 1 ], [ 3 ] ]. When it should be
[ [ 1, 1, 1 ], [ 1, 2 ], [ 3 ] ].

So my problem is I don't know how to jump out of the recursion and not explore the the full breadth of a particular branch. If that makes sense. Any hints would be appreciated.

Return consistent types

Both functions may return different types. For example combos may return an array or undefined. It would be better to return consistent type from all execution paths.

Input validation

The sum function does some input validation, checking if its parameter is an array. But this is too late. It's better to do input validation as close to where you receive the input as possible.

Also, there's no need to require nonempty input. If the input is empty, your algorithm will correctly return 0 as sum or empty array for combos, and that's all well.

Algorithm

In the current algorithm it's inefficient that the sums of arrays are recomputed every time you add a new element, when you could easily know the sums so far. It would be better to use that knowledge and avoid unnecessary recomputations.

To avoid creating results that are duplicates, your algorithm would need a substantial rewrite, which is off-topic here.


Character to decimal conversion This converts from character representation of integers to their decimal value:

def chrtodec(str):
    dec = 0
    base = ord('0')
    for chr in str:
        dec = dec * 10 + ord(chr) - base
    return dec

For example: "123" -> 123

You shouldn't use chr or str as names because they shadow the builtin chr and str methods. If you then wanted to use str() you'd be out of luck because str is now a string instead of a function. Given that your function deals with both strings and character ordinals it is not unlikely that these could be used. string is a potential improvement, it can occasionally cause trouble if you're trying to use the string module of the same name.

You should also add comments and a docstring. Docstrings are basically comments that are programmatically accessible so that other users can understand how to use your function.

def chrtodec(str):
    """Converts a string to a float using character ordinal positions."""

I think you have another big problem though, which is invalid input handling. Take a look at these:

>>> chrtodec("hello")
619663
>>> chrtodec("12.21")
11821
>>> chrtodec("523.32f")
5228374

Clearly there are problems here when you have characters other than numbers, so I think what you should do is raise a ValueError when an invalid string is passed. You already have the tools to do this figured out of course if you just check that a character's ordinal fits in the right range.

if not (ord('0') <= ord(chr) <= ord('9')):
    raise ValueError("Invalid character {} in string {},"
                     "only digits are parseable.".format(chr, str))

You could also just use chr.isdigit() as @Barry pointed out.

Also you could add support for negative numbers by checking at the start if the first character is ord('-').

negative = ord(str[0]) == ord('-')

This evaluates the expression str[0] == ord('-') and sets negative as the boolean result. Note that to make this compatible with the error handling I suggested, you should then remove the first character from str. And probably update the error message and docstring too.

if negative:
    str = str[1:]

Then just return with a ternary that checks negative.

return dec if not negative else -dec


DnD dice roll parser Some time ago I've written a small parser (about 250 LoC) which is capable of executing the four arithmetic operators +-*/ as well as a dice-roll operator, NdM, which "rolls a dice", DnD style. The source contains only integers, but of course due to division, not only integers are handled. Floating-point errors are silently ignored and mocked.

Now that I've come across it again today, I feel a bit meh about it. It's not horrible, but it definitely could be better.

Since it's a bit longish, I'll only paste here the two functions I feel most uncomfortable about: The one which goes through the token lists and decides which to execute, and the one which actually executes the operators.

To clear up some details if you don't wish to see the entire source: The parser tokenizes input left->right, inserting numbers and operators into numberStack and operatorStack respectively. The "executer" (don't have a clue what's the proper name for it) goes right->left on the operator stack, and uses the simple operator-precedence logic to see which ones to execute. All these functions exist on one object. The rest should be straightforward from the variable/property names.

execute : function () {
    var idx;

    while ( (idx = this.operatorStack.length) ) {
        //execute, BACKWARDS! OH THE INSANITY
        while ( 0 <=-- idx ) {
            execute.call( this, this.operatorStack[idx], idx );
        }
    }

    function execute ( token, index ) {
        var last = this.operatorStack[ index + 1 ];

        //last one is more important than we are
        if ( last && last.precedence > token.precedence ) {
            //execute it
            this.operate( index + 1 );
        }
        //we're about to finish and the last one isn't as all-mighty as we
        // thought
        else if ( !index ) {
            //execute za operator!
            this.operate( index );
        }
    }
}

//snip

operate : function ( index ) {
    //grab the two numbers we care about
    //since the source string looks like: 2 + 1
    // and the index param is actually the index of the operator to use,
    // we grab the index-th number and the index-th+1 number
    //in the above example, index = 0, we grab numberStack[0] and
    // numberStack[1]
    var couplet = this.numberStack.slice( index, index + 2 );
    //in addition to the numbers we operate on, there's also a dice-roll
    // operator, so we take it into consideration
    couplet.push( this.rolls );

    //arr.splice removes items and returns the removed items as an array
    //we remove the index-th item from the operatorStack and grab its
    // "value", which is the operator symbol (+, * etc)
    //when we have that value, we grab the corresponding operator object
    var op = operators[ this.operatorStack.splice(index, 1)[0].value ];

    //arr.splice, as well as removing items, can also add items
    //so, we slice-n-dice at the two numbers, grab the result of executing
    // the operator, and add that result where we finished slicing
    //for example:
    // [0, 1, 2].splice( 0, 2, 42 )
    //will make the array look like
    // [42, 2]
    this.numberStack.splice( index, 2, op.exec.apply(null, couplet) );
}

Full code can be found here: https://gist.github.com/1761880

There are a couple of issues I notice when I look at the code style of your code.

You could hide your internal variables by using the module pattern. Information hiding is a very important concept. Although you have hidden the fields already by putting everything in a function, this will allow more fine grained control. It will also remove the need to use the call function on inner functions.

There are two functions that have the same name execute (parser.execute and the function within). This confusing. It would be better if you would pick two unique names for that.

The names execute and operate are very generic and these two words have a similar meaning. By looking at the names I cannot determine what these functions do. A more clear name would be better.

Abbreviating variables is confusing. Using idx instead of index isn't a real saving, since you should optimize for reading, not for writing. (You will read your code more often than you write it) index is still vague. Describing what kind of index it is makes the variable name more clear.

Comments like

//execute it

are useless. You shouldn't use comments for these straight forward things. Explaining how the other functions work, as is done with array.splice, is also something that isn't common to do.

Using

else if ( !index ) {

for checking whether a index is 0 is confusing. Better would be

else if ( index == 0 ) {

, because then it is instantly clear you are checking whether it is zero instead of not initialized.


Exceptions for control flow I just wrote the following javascript (jQeury loaded):

var valid, ui;

try {
    $.each(this._cache, function() {
        $.each(this, function() {
            $.each(this, function() {
                if (this.label.toLowerCase() === valueLowerCase) {
                    valid = true;
                    ui = {item: $.extend({}, this) };
                    throw 'Control flow throw, please rethrow if accidently caught';
                }
            });
        });
    });
} catch(e) {}

My coworker suggested I rewrite this as follows:

var valid, ui;

$.each(this._cache, function() {
    $.each(this, function() {
        $.each(this, function() {
            if (this.label.toLowerCase() === valueLowerCase) {
                valid = true;
                ui = {item: $.extend({}, this) };
            }
            return !valid;
        });
        return !valid;
    });
    return !valid;
});

I think this is a valid case of using exceptions as control flow. Which way is the correct way to do this?

As Jef Vanzella mentioned, exceptions are for unexpected bad things, not for jumping out of control flow. Hence the second option is better, but it is still substandard code.

Instead of using this, you should have a meaningful parameter name in your function declaration.

Also, you could simply return the falsey !ui instead of !valid.

Finally, not many people know that returning false stops the iteration, that deserves a comment.

Now imagine that your data structure was Cache -> Orders -> Shipments -> Items then you could write:

var ui;

//Returning false to each() will stop the iteration
$.each(this._cache, function( order ) {
    $.each(order, function( shipment ) {
        $.each(shipment, function( item) {
            if (item.label.toLowerCase() === valueLowerCase) {
                ui = {item: $.extend({}, this) };
            }
            return !ui ;
        });
        return !ui ;
    });
    return !ui ;
});


Approximating square root (loops and functions exercise) Just looking for some feedback on my solution for Tour of Go Exercise 24 - Loops and Functions. Is there anything that looks like a wrong way to do things in go? anything I could have done better?

package main

import (
    "fmt"
    "math"
)

func Approx(x float64, z float64) float64 {

    return z - (((z * z) - x) / (2 * z))

}

func Sqrt(x float64) float64 {

    previous := 0.0
    delta := 1.0
    z := x

    for delta > 0.005 {

        z = Approx(x, z)

        if previous > 0.0 {
            delta = previous - z
        }

        previous = z
    }

    return z

}

func main() {

    for i := 1; i < 11; i++ {

        value := float64(i)

        fmt.Println("Calculating Sqrt for ", value)

        real := math.Sqrt(value)
        approx := Sqrt(value)

        fmt.Println("Real ", real)
        fmt.Println("Approx ", approx)

    }

}

One tiny thing: If you drop one set of parentheses:

return z - ((z * z - x) / (2 * z))

and then run go fmt, it will change it to

return z - ((z*z - x) / (2 * z))

The expressions around the * are close to the * and the expressions around the - are spaced out. In other words, go fmt implies the correct precedence.

Also, using line breaks to separate "sections" of a function is good, but there's no need to add an extra linebreak at the very beginning of a function or just before a }.


Newton's algorithm for a polynomial of arbitary degree Improvement over the last one I posted (now deleted, had no answers and was only cubics). Uses Horner's algorithm this time...

package main

import (
    "fmt"
    "math"
)

const ε float64 = 0.01

type polynomial struct {
    coeffs []float64
    x      float64
}

// Both of these functions use Horner's algorithm

func evalPoly(p polynomial, n int) float64 {
    var result float64 = 0
    for i := 0; i < n; i++ {
        result = result*p.x + p.coeffs[i]
    }
    return result
}

func evalPolyDeriv(p polynomial, n int) float64 {
    var result float64 = 0
    for i := 0; i < n-1; i++ {
        result = result*p.x + float64(n-i-1)*p.coeffs[i]
    }
    return result
}

func f(p polynomial) float64 {
    return evalPoly(p, len(p.coeffs)) / evalPolyDeriv(p, len(p.coeffs))
}

func solveNewton(p polynomial) float64 {
    h := f(p)
    for math.Abs(h) >= ε {
        p.x -= h
        h = f(p)
    }
    return p.x - h
}

func main() {
    guess := float64(2)
    coeffs := []float64{1, -2, 1, -4} // Can be of arbitary length
    fmt.Println(solveNewton(polynomial{coeffs: coeffs, x: guess})) // 2.3146...
}

In general, LGTM. Few notes:

The name f is meaningless. newtonDelta perhaps?
It feels right to compute a derivative's coefficients once, and reuse evalPoly for both the function and its derivative. After all, the polynomial's derivative is also polynomial. There is an usual space-time tradeoff, but in the case of polynomials DRY rule casts a deciding vote.
Newton's algorithm does not necessarily converge. You should be prepared to handle the divergent case.
Hardcoding ε is dubious. I recommend to solveNewton have it as a parameter.
Using Horner schedule is a definite improvement.


Efficient Collatz sequence analysis I'm new to Haskell, and I'm wondering why my programs are so slow compared to other languages.

Haskell, 6 seconds (x64, -O2):

nextCollatz :: Int -> Int
nextCollatz x = if even x
    then quot x 2
    else 3 * x + 1

collatzLength :: Int -> Int
collatzLength x = if x == 1
    then 0
    else 1 + collatzLength (nextCollatz x)

main = print . show . sum . map collatzLength $ [1..3000000]

Julia, 0.8 seconds (excluding the compilation time):

function main()
    sum = 0
    for i = [1:3000000]
        while i > 1
            i = isodd(i) ? 3 * i + 1 : i >> 1
            sum += 1
        end
    end
    sum
end

println(main())

Maybe the comparison isn't fair, but I'd like to know how to improve my Haskell code to bring it on par with other high-level languages such as Julia and JavaScript.

While memoization and parallelization would definitely help, currently I'm concerned with efficient iteration and arithmetic.

Your initial version - 6.834 seconds.

Version with quot and even operations replaced with bitwise - 6.651 seconds.

This version - 0.928 seconds:

import Data.Bits

tOdd sum 1 = sum
tOdd sum x = tEven (sum + (tEven2 0 x)) (x - 1)

tEven sum x = tOdd (sum + (tOdd2 0 x)) (x - 1)

tEven2 sum x | (x .&. 2) == 0 = tEven2 (sum + 1) (x `shiftR` 1)
tEven2 sum x | otherwise      = tOdd2 (sum + 1) (x `shiftR` 1)

tOdd2 sum 1 = sum
tOdd2 sum x = tEven2 (sum + 1) (3 * x + 1)

main = print . show $ tEven (0 :: Int) (3000000 :: Int)

The previous with -O3 -fllvm -optlo-O3 flags - 0.686 seconds.

The previous with all functions supported with INLINE pragma - 0.604 seconds.

More than 10 times faster than initially :)

Yes, low-level numeric operations and iterations are hard in Haskell, the language doesn't suite well for this.


Test if a number is within 2 of a multiple of 10 I have solved this exercise, but it looks really redundant. Is there a built-in function in Java or a cleaner way to caluclate mod of a value + deviation without multiple OR statements? If I had to calculate this for a deviation of 5, this would then require a lot of repetition.

  Given a non-negative number "num", return true if num is within 2 of a multiple of 10. Note: (a % b) is the remainder of dividing a by b, so (7 % 5) is 2.

    public boolean nearTen(int num) {
        return isMod10(num) || isMod10(num - 1) || isMod10(num + 1) || isMod10(num - 2) || isMod10(num + 2);
    }

    private boolean isMod10(int num) {
        return num % 10 == 0;
    }

You should read about congruent integers and modular arithmic. The distance of a number to a base is the minimum distance of its congruent value and its inverted congruent value.

 public boolean nearBase(int num, int base, int deviation) {

        // ..TODO check guards, normalize input or throw exceptions
        // - base and deviation are expected strict positive integers
        // - deviation is expected smaller than base
        // - num is expected a positive integer

        var congruent = num % base;
        var inverted = base - congruent;
        var distance = Math.min(congruent, inverted);
        return distance <= deviation;
    }

 // your method rewritten ->
 public boolean nearTen(int num) {
        return nearBase(num, 10, 2);
    }


Sieve of Eratosthenes solution for CodeEval The code below takes integer n as input, and delivers a list of all primes up to integer n using the Sieve of Eratosthenes.

My question is, could you please help me optimize this code? Is it considered poor code to use 'try' flow control? I'd like to optimize the code such that I don't need it.

def factorfinder(n):
    A = range(0,n+1)
    for i in xrange(0,int(math.sqrt(n))):
        if i == 0 or i == 1:
            A[i] = 0
            continue
        for j in xrange(0,n):
            try:
                A[i**2+j*i] = 0
            except IndexError:
                pass
    return filter(lambda x: x != 0, A)

Your if statement can be moved out of the loop, if you do you need to change the range to start at 2.

A[0] = 0
A[1] = 0
for i in xrange(2, int(math.sqrt(n))):

For your other loop, we can think of the algorithm slightly differently.
You start at i ** 2, you take a step of i, and it ends on len(A) (shown by the try-except).
Or:

for j in xrange(i ** 2, n + 1, i):

As for filter you can change it to use bool or None instead of the lambda.
Alternately you can change it to a list comprehension:

return [i for i in A if i != 0]

This makes the code simple:

def factorfinder(n):
    A = range(n + 1)
    A[0] = 0
    A[1] = 0
    for i in xrange(2, int(math.sqrt(n))):
        for j in xrange(i ** 2, n + 1, i):
            A[j] = 0
    return filter(bool, A)


Creating lists and sublists dynamically I'm working on a simple little one page website for giving a presentation and I have some working code now, but I'm thinking there has to be a more elegant way of doing this. I would describe myself, at best, as a "dabbler" in JavaScript at this point; I'm much more comfortable with statically-typed languages and am doing this as a learning exercise.

Here's my JSFiddle showing what I am doing.

In my "real" site, the PageContent object is coming from an AJAX POST response (via an instance of node.js). I've included all of that code to make it easier to understand what is going on (any suggestions on how to restructure that server-side code would also be appreciated). If I have the time, I'll probably read these "points" from a config file or something else, but for now, a switch statement fits my needs.

The segment of this that makes me the most uneasy is this:

$('#pageContent').append('
' + mp.main);
    if (mp.subPoints.length > 0){
        $('li#'+i).append('
');
        $.each(mp.subPoints, function(index, sp){
            $('li#'+i+' ul').append('
' + sp + '
');
        });
        $('li#'+i).append('
');
    }
    $('#pageContent').append('
');

I don't like how I am having to add id values to my list items so I can then append my subpoints to the correct parent. Is there a less hacky-feeling way to go about this?

In case it isn't obvious from the fiddle, the page should show a list of main points, and, if they exist, subpoints under each main point. I'm basically trying to mimic PowerPoint.

Any criticisms of this code would be gladly accepted; I know this probably isn't idiomatic JavaScript.

This will depend on your content and how you prepare it, but I'd suggest a very generic solution that'll work for any level of nested points. Now, arbitrary nesting doesn't appear to be required in your case, but, hey, nice to have.

Suppose your JSON content is structured like so:

var points = [
    {title: "Point", children: [
        {title: "Point"},
        {title: "Point"},
        {title: "Point"},
        {title: "Point", children: [
            {title: "Point"},
            {title: "Point"},
            {title: "Point", children: [
              // more...?
            ]}
        ]}
    ]}
]

You'll note that you can just keep nesting the points indefinitely.

To render this as HTML, you can use a recursive function. Like this:

function buildList(parentElement, items) {
    var i, l, list, li;
    if( !items || !items.length ) { return; } // return here if there are no items to render
    list = $("

").appendTo(parentElement); // create a list element within the parent element
    for(i = 0, l = items.length ; i < l ; i++) {
        li = $("

").text(items[i].title);  // make a list item element
        buildList(li, items[i].children);          // add its subpoints
        list.append(li);
    }
}

And call it like so:

buildList($("#pageContent").empty(), points);

The point is that since the structure is recursive, it can nest to any depth, but the code is simpler.

Here's a jsfiddle. This is quite a different approach, and Daniel Cook's answer is probably more immediately applicable to you current code, but I thought it worth to point out.

You could also extend it to add the "1", "1.1", "1.2" (and so on) numbers to the titles.


Mount different remote systems I have a shell script to mount different remote systems. While it does work, the area I struggle with (not being a programmer) is the logic of the script: it feels quite stilted and there seems to be some unnecessary duplication.

I would appreciate any comments as to how I could improve it - both in terms of the logic, but also any other tips about style or approach.

#!/bin/sh
# mount boxes by SSHFS

usage () {
    cat <<EOF
    sshmnt -[c,s,u,h]
      -c Box1
      -s Box2
      -u unmount
      -h print this message

EOF
}

mnt(){
    sshfs jason@"$host":/home/jason /media/"$dir" \
    -C -p "$port" \
    -o reconnect,IdentityFile=/home/jason/.ssh/id_rsa \
    && ls /media/"$dir"
}

umnt(){
    fusermount -u /media/"$dir"
}

# unmounting
if [ "$1" = "-u" ]; then
    box=$(mount | grep Box1)
    if [ -n "$box" ]; then
        dir=Box1
    else
        dir=Box2
    fi
    umnt && exit
fi

# check if on LAN
lan="$(ip addr | grep .102/)"

if [ -n "$lan" ]; then
    case "$1" in
      -c) int=100
          ;;
      -s) int=200
          ;;
    esac
    host="192.168.1.$int"
else
    host="XXX.XXX.XXX.XXX"
fi

# box specifics
case "$1" in
  -c) dir=Box1 port="XXXX"
      mnt
      ;;
  -s) dir=Box2  port="XXXX"
      mnt
      ;;
   *) usage && exit
       ;;
esac

I have obscured the relevant ports and hostnames.

Your functions shouldn't depend on variables but use specified
parameters. You always specified variables before calling your function instead of just specifying them as parameters, e.g:

DIR=Box1
Host=192.168.1.100
mnt

but you can easily write it as

mnt Box1 192.168.1.100

I would also only parse your command line arguments in one place and create
a dedicated function for determine the appropriate ip address for your hosts.

I would also remove the ls call from your mnt function as mnt doesn't do what the name suggests. But this is a rather minor issue and I didn't change it in my code.

#!/bin/sh
# mount boxes by SSHFS

usage () {
    cat <<EOF
sshmnt -[c,s,u,h]
      -c Box1
      -s Box2
      -u unmount
      -h print this message

EOF
}

mnt(){

    if [ $# -ne 3 ] ; then
       echo "Wrong parameters for mnt - host dir port" >&2
       return 1
    fi

    host="$1"
    dir="$2"
    port="$3"
    sshfs jason@"$host":/home/jason /media/"$dir" \
    -C -p "$port" \
    -o reconnect,IdentityFile=/home/jason/.ssh/id_rsa \
    && ls /media/"$dir"
}

umnt(){
    dir="$1"
    fusermount -u /media/"$dir"
}

get_host(){
    # check if on LAN
    lan="$(ip addr | grep .102/)"

    if [ -z "$lan" ] ; then
        echo XXX.XXX.XXX.XXX
        return 0
    fi

    case "$1" in
        Box1) echo 192.168.1.100 ;;
        Box2) echo 192.168.1.200 ;;
        *) echo "Unknown Parameter" >&2 && return 1 ;;
    esac
}

case "$1" in
    -c) mnt $(get_host Box1) Box1 port ;;
    -s) mnt $(get_host Box2) Box2 port ;;
    -u)
        mount | grep Box1 && umnt Box1
        mount | grep Box2 && umnt Box2
        ;;
     *) usage && exit 1;;
esac

Another suggestion is to get rid of all your hard coded paths and texts like, /media/, /home/jason/ and jason and replace them with variables, e.g:

MOUNT_PREFIX=/media/
SSH_KEY=/home/jason/.ssh/id_rsa
MOUNT_SOURCE=/home/jason

and just specify the user for your hosts in ~/.ssh/config


Searching a string with text search say that:

var stringToTest = "Hello i am walking"

And

var searchText = "Hello walking"

I have the following javascript function for filtering.

var searchSplit = searchText.split(" ");
var num_split = searchSplit.length;
var correctTested = 0;
searchSplit.forEach(function (testString) {
    if (stringToTest.indexOf(testString) >= 0) {
        correctTested++;
    }
});

//Decides if the value are close enough
if (correctTested > 0) {
    var percentageCorrect = correctTested / num_split * 100;
    return percentageCorrect >= 50;
}
else {
    return false;
}

I have never done anything like this and i would like if you would rate or even come with suggestions on how to improve the above code.

My suggestions are mostly focused on making the code as concise and clear as possible.

Simplify the return statement.

if (correctTested > 0) {
    var percentageCorrect = correctTested / num_split * 100;
    return percentageCorrect >= 50;
} else {
    return false;
}

can be reduced to

return correctTested >= num_split * 0.5;

This eliminates both the condition as well as concern about div by 0.
Since you do not need a post-increment, choose pre-increment.
++correctTested or correctTested += 1 rather than correctTested++.
Choose clear names.

searchSplit might be clearer as searchTokens, correctTested as matchedTokens, num_split as tokenCount.
Consistent names.
searchCase is camel case while num_split is snake case.
You have not articulated the functional spec, but consider whether the search should be case-insensitive.  Also consider whether partial word matches are in fact acceptable or whether you require full token matches.

For ease of testing and for the ability to add different decision criteria you could consider decomposing match counting and the decision criteria into two separate functions.