Verifying if a string can be created with the letters of another string This is basically based on the following question on stackoverflow which was based on a codewars assignment that can be found here
Problem statement

Complete the function scramble(str1, str2) that returns true if a portion of str1 characters can be rearranged to match str2, otherwise returns false.

Notes:

Only lower case letters will be used (a-z). No punctuation or digits will be included.
Performance needs to be considered

My solution
function scramble(str1, str2) {
  if (str1.length < str2.length) {
    return false;
  }
  const set = new Set([...str2]);
  return Array.from( set ).every( letter => str2.split(letter).length <= str1.split(letter).length );
}

As with the original question on SO, my question would be if there are more efficient ways of completing the assignment. I am mainly worried about the string.split, but I didn't know of any other method that might be faster (I know I should substract 1 to get the actual letter count, but it doesn't matter for this usecase)

Your logic seems hard to follow.  Why spread str2 to array, which you then read into a Set, only to turn around and make the Set into an array, which you then iterate, comparing each letter’s count in each string by splitting them repeatedly?
You see how long and convoluted that sentence was? The last line of code there is kind of the same, dense and hard to follow.
The problem should be solved by counting characters for str1 into map keyed by each letter. You then iterate through str2 and decrement those counters. If any counter goes to -1 you have false condition.  This approach means each letter in passed string only needs to be visited once making performance linear to the combined length of the strings in operational complexity.  Your current solution has exponential complexity.


Scrolling background in kivy I'm drawing a background into kivy, and then wrap and place a transparent image on top of it. Each second I move the location of the transparent a little bit.

from kivy.app import App
from kivy.uix.widget import Widget
from kivy.properties import ObjectProperty
from kivy.uix.image import Image
from kivy.clock import Clock

class ScrollApp(App):

    def build(self):
        self.bkg = Background()
        self.clock = Clock.schedule_interval(self.bkg.scroll_texture, 1)
        return self.bkg

class Background(Widget):

    plx_2 = ObjectProperty(None)

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        # set as texture
        self.plx_2 = Image(source=r"assets\plx-2.2.png").texture
        self.plx_2.wrap = 'repeat'
        self.plx_2.uvsize = (1,-1)

    def scroll_texture(self, time_passed):
        # update uvpos
        x,y = self.plx_2.uvpos
        x = x - 0.05 % 1 # this magic number will change later
        self.plx_2.uvpos = (x, y)
        # redraw the image
        texture = self.property('plx_2')
        texture.dispatch(self)

if __name__=='__main__':
    ScrollApp().run()

And the .kv file:

#:kivy 1.11.1

<Background>:
    canvas.before:
        Rectangle:
            size : self.size
            pos : self.pos
            source: "assets\plx-1.png"
        Rectangle:
            size : self.size
            texture: self.plx_2

The code works, but I feel very odd about the way I'm redrawing the texture with dispatching itself as an event. Is there a better way? Second, now if I were to add multiple transparent images on top, I would have to add a lot of code. Is there a a better way than just adding another Rectangle in the .kv file, adding another ObjectProperty in the Background class etc, etc.?

The only really odd thing I see is this:

x - 0.05 % 1

Modulation takes precedence over subtraction, and 0.05 % 1 == 0.05, so the modulation has no effect. Is this what you intended?


Handle static objects using custom adapter I have read some articles discussing about why static is bad, some of them refering static as not-testable, non-mockable, has state (non-stateless) and everything else.

However I have encountered some cases where I am forced to use static implementation, such as:

During using HttpContext.Current in Asp.Net, as well as Session
The HttpContext and Session sometimes contains configurations, which is needed to do some validations
Accessing some static objects in framework or third-party component
Maybe there are other situation where I don't yet encountered

Up until now I have handle those static methods using custom adapters (correct me if I misuse the term adapter here, or maybe it is better for provider), such as:

public class ConfigurationAdapter : IConfigurationAdapter
{
    public string CurrentUsername
    {
        get
        {
            return HttpContext.Current.User.Identity.Name;
        }
    }
}

Then the consumer will use the adapter like:

public class ConfigurationConsumer : IConfigurationConsumer
{
    public ConfigurationConsumer(IConfigurationAdapter adapter)
    {
        _adapter = adapter;
    }
    private readonly IConfigurationAdapter _adapter;

    public void DoSomething()
    {
        string userName = _adapter.CurrentUsername;
    }
}

I believe that this way I can get the benefits:

The adapter can be replaced by other implementation
The adapter can be mocked
The consumer does not directly dependent on HttpContext

However due to inexperienced in design, I am still wondering about:

Does this kind of implementation can really decouple the consumer with the static object?
Will it provide any problem in the future?
Is there any better design or pattern than this?

In this case I want to make sure that my application is testable, maintainable dan extendable. I believe performance is not effecting too much here, so it can be ignored.

Any kind of thoughts will be appreciated

I see no problem in such wrappers except they are kinda cumbersome to write. They provide a good way to decouple your code not only from static objects, but from the all the objects you cannot mock too (for example, sealed classes from 3rd party libraries). I use that kind of adapters myself and have little problem with them. It is important to have these classes as simple as possible (so that you don't need to test them) and only expose properties and methods you realy need.

Update: ctrucza suggests using a service locator in his answer. While it's a good solution when your class has many dependencies which leads to many constructor parameters, it makes unit-testing your class a little harder.

If you add some new dependency into your class and resolve it with some generic service locator's method then your code (including unit tests and production code) will compile just fine. You only realize that something is wrong when your working unit-tests will suddenly fail with null reference exception, or some other resolution exception your service locator provides. The constructor injection, on the other hand, gives you a clear knowledge of what dependencies a class using and if you miss some constructor parameter in your tests, the compiler will tell you.

One more thing. If you have a dozen dependencies in your class, may be the design of the class is wrong and refactoring is needed. I think that's really good indicator, which will be hidden if you are using a service locator pattern.


Finding the shortest path in a graph Could you please advise on the code and its design? Is there any way I can apply object oriented principles to make the design robust yet flexible?

public class RouteFinder{

    /** Used to compute shortest distance from source station to all other station in a given network.
     * @param source source station from which shortest distance will be calculated to the stations
     */
    public void computePaths(Vertex source)
    {
        source.minDistance = 0.;
        PriorityQueue<Vertex> vertexQueue = new PriorityQueue<Vertex>();
        vertexQueue.add(source);
        while (!vertexQueue.isEmpty()) {
            Vertex u = vertexQueue.poll();
            // Visit each edge exiting u
            for (Edge e : u.adjacencies)
            {
                Vertex v = e.target;
                double weight = e.weight;
                double distanceThroughU = u.minDistance + weight;
                if (distanceThroughU < v.minDistance) {
                    vertexQueue.remove(v);
                    v.minDistance = distanceThroughU ;
                    v.previous = u;
                    vertexQueue.add(v);
                }
            }
        }
    }
    /** Used to return the shortest path from a source to destination station.
     * @param target destination station
     * @return shortest path from source station to destination station.
     */
    public static List<Vertex> getShortestPathTo(Vertex target)
    {
        List<Vertex> path = new ArrayList<Vertex>();
        for (Vertex vertex = target; vertex != null; vertex = vertex.previous)
            path.add(vertex);
        Collections.reverse(path);
        return path;
    }
}

You mentioned OOP, and thats great, and OOP is all about code reusability and extensibility,and this code is a good example of a Candy Machine Interface. And that is, users of this class might do mistakes while using it. users could call the static the method getShortestPathTo before calling instance method computePaths and this would lead to wrong routes because weights weren't computed properly. You should try your best to force right behavior.
class RouteFinder{

   RouteFinder(Vertex source){
     computePaths(source);
   }
   private computePaths(Vertex source){
     //It is private,users dont need to know about this
   }

  public List<Vertex> getShortestPathTo(Vertex target){
    //..
   }
}

As you can see here no static methods anymore, static usually presents a global behavior, and I need to force users to go through the constructor so I make sure weights are computed properly.
I suppose that what you using is Dijkstra algorithm (not sure though), where your class name is RouteFinder, that's a bit misleading because Dijkstra is not the only algorithm and it doesn't work with negative weights, and you need something like Bellman-Ford for that.If you don't want to support negative weights for now, it is fine, but you should be able to add this functionality in the future.And this leads for extracting the Route finder to an interface
 interface RouteFinder{
   List<Vertex> shortestPath(Vertex target);
 }

 class Dijkstra implements RouteFinder{
   ...
 }

 class BellmanFord implements RouteFinder{
   ...
 }

And it would be great if you just expose the interface and keep concrete implementation with package access, a good API is a compact one. And for that you can have a static factory to get the right implementation
public static RouteFinder getInstance(Vertex source){
 if(hasNegativeWeight){
  return new BelmmanFord(source);
 }else{
  return new Dijkstra(source);
 }
}

One more thing, is it possible to have duplicated vertices on the same graph? I dont think so, and hence getShortestPathTo should return a Set of vertices rather than a List


Checking whether various form fields are filled in This is a customer information data entry form. First name and Last name are required. In addition phone number, e-mail and birth date are used to check the database for existing customers with the same information.

Either of these fields (phone, e-mail or birth date) can be empty or filled in. In order to validate against database I need to check if the user has filled each field in or not.

Is there a way to refactor the code and still set a different property for each scenario?

The code looks like this:

var sFirstName = Inputs.GetProperty("sFirstName");
var sLastName = Inputs.GetProperty("sLastName");
var sEmail = Inputs.GetProperty("sEmail");
var sPhone = Inputs.GetProperty("sPhone");
var sBirthDate = Inputs.GetProperty("sBirthDate");
var sCoAddress = Inputs.GetProperty("sCoAddress");
var sAddress = Inputs.GetProperty("sAddress");
var sZip = Inputs.GetProperty("sZip");
var sCity = Inputs.GetProperty("sCity");

if(!sFirstName || !sLastName){
    TheApplication().RaiseErrorText("First name and last name are required");
}

if(!sEmail && (!sCoAddress || !sAddress || !sZip || !sCity)){
    TheApplication().RaiseErrorText("Please enter either e-mail or postal address");
}
if(sEmail && sBirthDate && sPhone){
    Outputs.SetProperty("QueryAll","Y");
}
else if(sEmail && sBirthDate && !sPhone){
    Outputs.SetProperty("QueryEmailBday","Y");
}
else if(sEmail && !sBirthDate && sPhone){
    Outputs.SetProperty("QueryEmailPhone","Y");
}
else if(!sEmail && sBirthDate && sPhone){
    Outputs.SetProperty("QueryBdayPhone","Y");
}
else if(sEmail && !sBirthDate && !sPhone){
    Outputs.SetProperty("QueryEmail","Y");
}
else if(!sEmail && sBirthDate && !sPhone){
    Outputs.SetProperty("QueryBirthDate","Y");
}
else if(!sEmail && !sBirthDate && sPhone){
    Outputs.SetProperty("QueryPhone","Y");
}
else if(!sEmail && !sBirthDate && !sPhone){
    Outputs.SetProperty("QueryNameOnly","Y");
}

You could solve this by constructing your Queryname dynamically:

var queryName = "Query";
if ( sEmail ) queryName += "Email";
if ( sBDay  ) queryName += "BDay";
if ( sPhone ) queryName += "Phone";

if ( queryName == "Query" ) queryName = "QueryNameOnly";

Outputs.SetProperty( queryName,"Y" );

But for this you would have to be a little more strict with your Parameter-Names not changing between BDay and BithDate and no Specialcase for ALL


Preventing resubmission of form using timestamp I am working on a method to prevent users from resubmitting a PHP form and I thought of many methods, but this one seemed to be the most reliable in my tests. I did not want to create redirects for development purposes, and transferring data around.

I create a hidden input with the form holding the time() of which the page loads. Then when the form is submitted I check for the time_stamp + int < time().

This is what it looks like in practice:

$unix = $_POST['unix_timeStamp'];
if($unix + 1800 > time())
{
    echo "You too long friend
";
    echo 'start time: '. $unix . '
';
    echo 'end time: '. time();
    exit;
}

It works fine, after 1800 seconds (30 minutes) the form will simply indicate that the time has expired and they have to start again.

I am just wondering if this is a fool-proof system. Not security wise, but just to prevent double-submits or users taking too long.

I see how this stops the user from taking too long to fill out the form (which is not necessarily something you want to stop), but I don't understand how this prevents double submits at all.  Typical reasons for double submits:

The user clicks submit, the website response slowly, they click submit again
After submitting, if no redirect happens (you said you don't want redirects), the person can hit back and be presented with "Must resubmit data". If they refresh on this screen a second submit will happen.

In both cases the second submit can happen just moments after the first, so I don't understand how a time-based limit can stop either kind of double submit.

In the past when I've wanted to prevent double submits without using redirects I've done so by keeping track of a unique key in the session everytime I displayed the form.  When the form came back up, the unique key came with it (in a hidden input like yours), and I marked that unique key as "used".  Anytime a submit came up I checked its key against the list of "used" unique keys.  If the key in the form was marked as used, then I knew it was a double submit, and returned an error to the user letting them know.

As an aside, the need to send out a unique key with each form happens to also be a requirement for CSRF protection, so I actually just rolled both bits of functionality into one and killed two birds with one stone.


Appending to file slowdowns the process The process:

I read the lookup and create a list aa.
I traverse through the glob.glob("C:\\project resource\\dump\\*.html") and find matching.
If a match is found, I create a file if not available, but if available, I just append it to the file.

import glob,re
from bs4 import BeautifulSoup
import unidecode
lis=[]
aa=open("C:\project resource\lookup.txt","r")
aa=aa.readlines()
aa=[b.split("\t") for b in aa ]
print "started"
for sa in aa:
    for a in glob.glob("C:\\project resource\\dump\\*.html"):

        if sa[0] =="".join(re.findall("_(\d+).",a)):
            try:
                fi= open(sa[1].strip()+".txt","a")
                html=" ".join(open(a,"r").readlines()[1:])
                soup = BeautifulSoup(html)
                # kill all script and style elements
                for script in soup(["script", "style"]):
                    script.extract()    # rip it out
                # get text
                text = soup.get_text()
                # break into lines and remove leading and trailing space on each
                lines = (line.strip() for line in text.splitlines())
                # break multi-headlines into a line each
                chunks = (phrase.strip() for line in lines for phrase in line.split("  "))
                # drop blank lines
                text = ' '.join(chunk for chunk in chunks if chunk)
                text=re.sub("\s+"," ",text)

                fi.write(unidecode.unidecode(text)+"\n")
            except:
                print a
            fi.close()
            break
    else:
        print sa,"no match"

lookup.txt sample:

123\tapple
124\tdad
125\tdude

Sample file names in dump:

12_123.html
12_124.html
123234_125.html

Time:

File count :1,65,000
Time: 2 days

Is there a change which can increase the speed of this process?

Imports should be on different lines.

import glob
import re

'lis' should be removed, due to it not being used.
If you did use it, it should be renamed, a descriptive name is the best.

when opening a file use with.

with open(...) as aa:
    ...

This automatically closes the file, even if there is an unhandled exception. So if for example you keyboard interrupt the program, the file will close.

Also change the variable name aa to something better. file_handler is better.

You overwrite aa. This means that you cannot close it.
You should always close a file handler.

You could not overwrite aa by changing the for statement.

for sa in (b.split("\t") for b in aa.readlines()):

Python's style guide, PEP8, says that all operators should have a space either side.

fi = open(sa[1].strip() + ".txt", "a")

Currently you use three styles to, one of which is allowed.
The following one line shows two of them. fi= o )+"

fi= open(sa[1].strip()+".txt","a")

Your try statement is massive. It is also a bare except. Both being very bad.

You should try to keep the try as small as possible.
This is so that you don't 'mask' bugs.

try:
    file_handler = open(sa[1].strip() + ".txt", "a")
except:
    print "Error on opening " + a
else:
    ...
finally:
    file_handler.close()

You could remove the print statements. They take a surprising amount of time to print.

You should wrap you main in a if __name__ == '__main__':

Performance

Your massive amount of white-stripping may be the problem.
And you are opening a lot of files.

The former you can deal with.

text = soup.get_text()
lines = (line.strip() for line in text.splitlines())
chunks = (phrase.strip() for line in lines for phrase in line.split("  "))
text = ' '.join(chunk for chunk in chunks if chunk)

First off this can be simplified.

text = ' '.join(
    chunk
    for chunk in (
        phrase.strip()
        for phrase in line.strip().split("  ")
        for line in soup.get_text().splitlines()
    )
    if chunk
)

You strip every time you split. This can't be good.

It seems that you want to replace all blocks of white space with one space.
If this is true then the below function should be simpler and should be faster.

def strip_entire(string):
    def inner():
        white_space = False
        for char in string:
            if char in ' \t\n':
                if white_space:
                    continue
                white_space = True
                yield ' '
            else:
                white_space = False
                yield char
    return ''.join(inner())

It is \$O(n)\$. And should be faster than yours.

If you are likely to open the same file multiple times,
then you could save the output to another file.
Say a + ".stripped"

This will have the benefit of being able to reduce the amount of duplicate work.
This is good as then your computer can effortlessly append the file to the other file.

with open(sa[1].strip() + ".txt", "a") as file_handler:
    if os.path.exists(a + ".stripped"):
        with open(a + ".stripped", "r") as stripped_handler:
            file_handler.write(stripped_handler.read())
    else:
        # Other method

If you were to make all these changes you should get

import glob
import re
import unidecode
import os.path
from bs4 import BeautifulSoup

def strip_entire(string):
    def inner():
        white_space = False
        for char in string:
            if char in ' \t\n':
                if white_space:
                    continue
                white_space = True
                yield ' '
            else:
                white_space = False
                yield char
    return ''.join(inner())

def append_file(append_name, html_name):
    with open(append_name, "a") as file_handler:
        if os.path.exists(html_name + ".stripped"):
            with open(html_name + ".stripped", "r") as stripped_handler:
                file_handler.write(stripped_handler.read())
        else:
            with open(html_name, "r") html_handler:
                soup = BeautifulSoup(html_handler)
                # If you can't have the first line. No matter what.
                #soup = BeautifulSoup(''.join(html_handler.readlines()[1:]))
            for to_remove in soup(("script", "style")):
                to_remove.extract()
            file_handler.write(
                unidecode.unidecode(
                    re.sub("\s+", " ", strip_entire(soup.get_text()))) + "\n")

if __name__ == '__main__':
    with open("C:\project resource\lookup.txt", "r") as file_handler:
        print("started")
        dump_files = glob.glob("C:\\project resource\\dump\\*.html")
        for split_line in (b.split("\t") for b in file_handler.readlines()):
            for dump_file in dump_files:
                if split_line[0] == "".join(re.findall("_(\d+).", dump_file)):
                    append_file(split_line[1].strip() + ".txt", dump_file)
                    break

I don't fully understand what your regex's do.
As I don't fully understand the re library.
They seem kinda odd, re.findall("_(\d+).", dump_file),
I thought you only had one number on your files that followed that regex.
And so I decided to leave them as is.

I also removed the try except statement, as I didn't know what you were error handling. Was it your loops, your opens, everything, keyboard interrupts. This is one reason you shouldn't have bare except statements.

If you were doing the opens then you should do this:

try:
    file_handler = open(...)
except IOError:
    # Raise an error, break, return, whatever.
else:
    # Code in the with
finally:
    file_handler.close()


Cancel those hung print jobs I've created a program that will list all hung print jobs on a specified server, from there it will load them and strip the jobs down to their Job ID then cancel the jobs on that server only.

I would like a critique of my work please.

How does this look as an OOP script?
Is there anything I can do differently?
Is calling the Net::SSH.start twice really necessary?

Source:

#!/usr/local/bin/ruby

require 'rubygems'
require 'net/ssh'
require 'etc'

class PrintJobs

  HOST = ARGV[0]
  USERNAME = Etc.getlogin
  PASSWORD = nil

  def scan_for_jobs
    check_jobs = Net::SSH.start(HOST, USERNAME, :password => PASSWORD) do |ssh|
      cmd = "prt_jobs"
      info = ssh.exec!(cmd)
      if info == nil
        puts "No print jobs on server #{HOST}"
      else
        res = info.split("\n").reject {|line| line.match(/\s+2016\s+/)}.join("\n")
        puts res
        print "Process into kill que: "
        input = STDIN.gets.chomp.upcase
        if input == "YES"
          kill_que(check_jobs, res)
        else
          exit 1
        end
      end
    end
  end

  def kill_que(check_jobs, res)
    puts "Loading jobs into kill que.."
    column = 0
    job_ids = res.lines.map { |job| job.split(/\s+/)[0] }.each do |task_id|
      kill_jobs(task_id)
    end
  end

  def kill_jobs(task_id)
    execute = Net::SSH.start(HOST, USERNAME, :password => PASSWORD)
    id_to_strip = task_id.gsub(/\-/," ")
    column = 1
    stripping_id = id_to_strip.lines.map { |task| task.split(/\s+/)[1] }.each do |id|
      id.strip
      puts "Cancelling job: #{id}"
      #`sudo cancel #{id}`
      execute.exec!("sudo cancel #{id}")
    end
  end
end

killer = PrintJobs.new
killer.scan_for_jobs

Usage example:

closing_2-254128        root            933888   Thu 03 Dec 2015 03:49:13 PM CST
closing_2-254129        root            933888   Thu 03 Dec 2015 03:50:16 PM CST
laser42-254144          root            192512   Thu 03 Dec 2015 04:24:02 PM CST
laser42-254145          root            171008   Thu 03 Dec 2015 04:25:10 PM CST
ship_audit-254143       root            141312   Thu 03 Dec 2015 04:23:13 PM CST
Process into kill que: yes
Loading jobs into kill que..
Cancelling job: 254128
Cancelling job: 254129
Cancelling job: 254144
Cancelling job: 254145
Cancelling job: 254143

No print jobs on server

Wrapping your methods in a class definition isn't the same thing as making your project OOP. These methods don't care about their receiver, and forcing the programmer to instantiate the PrintJobs class before calling them doesn't accomplish anything useful here. Now, for special-purpose scripts like this, not being OO isn't the worst thing in the world - in fact, going OO can be design overkill for a project this small. However, if you go the functional route, tell it like it is - refactor class PrintJobs to module PrintJobs (because Module is, idiomatically, the correct type for a collection of associated methods) so that you can do your invocation with PrintJobs.scan_for_jobs.

If you want to convert the code to be more OO - to make it easier to incorporate into another project, for example - objects need to be responsible for the data they operate on. That means that the HOST/USERNAME/PASSWORD values need to be instance variables, not class constants. You also want to be sure that your data is typed appropriately. Currently, there's a lot of passing around and manipulation of strings that represent print jobs. If the output format of prt_jobs ever changes, we'll need to update every piece of code that interacts with these objects. Abstracting them into their own type will help prevent this.

Stylistic touches:

Get rid of extraneous definitions - cmd is defined and then only used once, in the immediately following line. Some other variables are defined and then never used.
Avoid abbreviation. I assume res means "results", but I'm not sure.
Exiting with a non-zero status code should only happen if the program encounters some sort of error, which isn't the case with your usage. I'd remove the statement entirely, since there's no real reason for PrintJobs to be responsible for program termination anyway.
Last but not least, the word is "queue".

Here's the class refactored, with additional comments in-line. Since I don't have a print server handy the code has not been tested and probably contains bugs, but it should give you an idea of what a more object-oriented structure looks like.

class PrintManager
  # I've made the PrintJob class belong to PrintManager for now,
  # but you could move it to global scope if it becomes handy elsewhere
  class PrintJob
    attr_reader :printer, :id, :user, :some_other_thing, :date

    def initialize(printer, id, user, some_other_thing, date)
      @printer, @id, @user = printer, id, user
      @some_other_thing, @date = some_other_thing, date
    end

    # If the output of `prt_jobs` ever changes, this is the only code you'll
    # need to update.
    # In real life we'd want to parse the time into a proper Time object.
    def self.parse(job_string)
      job_string =~ /^(\S+)-(\d+)\s+(\S+)\s+(\d+)\s+(.+?)\s*$/
      return self.new($1, $2.to_i, $3, $4.to_i, $5)
    end

    def to_s
      "#{@printer}-#{@id}\t#{@user}\t#{@some_other_thing}\t#{@date}"
    end
  end

  def initialize(host, username, password)
    @host, @username, @password = host, username, password
  end

  def scan_for_jobs
    Net::SSH.start(@host, @username, :password => @password) do |ssh|
      info = ssh.exec!("prt_jobs")
      if info == nil
        puts "No print jobs on server #{@host}"
      else
        # The old "res" object we got here was a big blobby String.
        # Now we have an Array of PrintJobs, which is much easier
        # to interact with.
        print_jobs = info.split("\n").map {|line|
          PrintJob.parse(line)
        }
        print_jobs.reject! {|job| job.date.match(/\s+2016\s+/) }

        # One disadvantage of parsing out the string at object
        # creation is that we lose formatting, and our tab stops
        # may not line up without some extra effort. Oh well!
        puts print_jobs
        print "Kill these jobs (yes/no): "
        input = STDIN.gets.chomp.upcase
        if input == "YES"
          # The kill_queue method only really existed to do string
          # parsing. We accidentally made it unnecessary when we made
          # PrintJob a real class.
          print_jobs.each { |job| kill_job(job, ssh)}
        end
      end
    end
  end

  # Since kill_job is only called from within scan_for_jobs, we'll
  # just reuse its SSH object. Depending on how your program grows, it
  # may be better to have kill_job open its own SSH connection like
  # you had originally, or make it an attribute of PrintManager, or
  # something else.
  def kill_job(job, ssh)
    # Got to remove a bunch more string parsing here
    puts "Cancelling job: #{job.id}"
    ssh.exec!("sudo cancel #{job.id}")
  end
end

killer = PrintManager.new(ARGV[0], Etc.getlogin, nil)
killer.scan_for_jobs


Scanning the subnet by ip range By applying the get_name and get_mac functions to the specified IP address, it is possible to estimate the network availability of a segment:

#include <Winsock2.h>
#include <iphlpapi.h>
#include <cstdio>

#pragma comment(lib, "iphlpapi.lib")
#pragma comment(lib, "ws2_32.lib")

bool get_name(unsigned char* name, char dest[32])
{
  struct in_addr destip;
  struct hostent* info;

  destip.s_addr = inet_addr(dest);

  info = gethostbyaddr((char*)&destip, 4, AF_INET);

  if (info != NULL)
{
  strcpy((char*)name, info->h_name);
}
else
{
return false;
}

return true;
}

bool get_mac(unsigned char* mac, char dest[32])
{
  struct in_addr destip;
  ULONG mac_address[2];
  ULONG mac_address_len = 6;

  destip.s_addr = inet_addr(dest);

  SendARP((IPAddr)destip.S_un.S_addr, 0, mac_address, &mac_address_len);

  if (mac_address_len)
  {
   BYTE* mac_address_buffer = (BYTE*)&mac_address;
   for (int i = 0; i < (int)mac_address_len; i++)
    {
    mac[i] = (char)mac_address_buffer[i];
    }
   }
   else
   {
     return false;
   }

    return true;
   }

   int main()
   {
     char address[][32] = { { "192.168.14.101" },{ "192.168.14.102" },
                     {"192.168.14.103" },{ "192.168.14.106 "} };
     WSADATA sock;

     if (WSAStartup(MAKEWORD(2, 2), &sock) != 0)
   {
     printf("Failed to initialise winsock. (%d)\n", WSAGetLastError());
     return 1;
   }

    for (int i = 0; i < (int)sizeof(address) / 32; i++)
    {
     unsigned char mac[6] = { '\0' };
     unsigned char name[100] = { '\0' };

     if (get_mac(mac, address[i]))
    {
      printf("%s : %s : %.2X-%.2X-%.2X-%.2X-%.2X-%.2X\n", address[i],
     (get_name(name, address[i])) ? (char*)name : "-",
     mac[0],mac[1],mac[2],mac[3], mac[4], mac[5]);
     fflush(stdout);
    }
    }

    printf("\nDone.\n");
    fflush(stdout);
    system("PAUSE");

   return 0;
   }

An attempt to implement a program that scans the subnet and displays IP address, host name, and MAC of machines on the network.

I won't cover any bugs in your implementation, I'll only have a look at your coding style. It looks like you were writing plain c but tagged it as c++. I think I didn't even found a single c++-only feature you made usage of.

Don't use a string as data storage unless it's necessary

And of course, don't use plain c-style strings, because you'll do it wrong.

char address[][32] = { { "192.168.14.101" },{ "192.168.14.102" },
                 {"192.168.14.103" },{ "192.168.14.106 "} };

You reserve 32 chars for your ip-address storage, but that isn't ever necessary. If you really want to store them as a string, do it as std::string, or if it's a constant string, as std::string_view(C++17). An ip-address has a upper maximum of 4 * 3 signs + 3 dot signs and additionally the zero at the end (that accumulates to a max of 16 signs). So, that means you reserved twice as much memory as you really ever need.

Use better data type

Why don't you simply store the ip-address as an 32bit uint; or as an std::array<std::unit8_t, 4>. You could easily create a typedef for that:
using ipv4 = std::array<std::unit8_t, 4>
With that typedef you'll be able to use that type everywhere you need the ipv4 address. That makes the code more readable and less error prone.

rethink your return statements

That looks really horrible.

bool get_name(unsigned char* name, char dest[32])
{
  // ...
  if (info != NULL)
  {
    // ...
  }
  else
  {
    return false;
  }
  return true;
}

You could write that in an easier way:

bool get_name(unsigned char* name, char dest[32])
{
  // ...
  if (info != NULL)
  {
    // ...
    return true;
  }
  return false;
}

use std::optional if you expect your function return a invalid return value

In C++17 we got a new helper template class called std::optional. It's very helpfull in returning values with an invalid state. You could adjust your get_name function like this:

std::optional<std::string> get_name(ipv4 ip)
{
  // ...
  if (info != NULL)
  {
    return info.name; // pseudo-code; don't know the real interface of this info object, but hopefully you get what I try to explain
  }
  return std::nullopt;
}

That will make your code more reable and self-descriptive. All of the above is also true for your get_mac function.

c-style cast are bad

You used them alot; don't do that. In c++ there are plenty of cast you should use instead. For example (int)mac_address_len should be written as static_cast<int>(mac_address_len)


A simple math torture game Introduction.

It is commonly known that the worst form of torture is performing monotonous tasks ad nauseam. So I constructed this simple game I can give to my students when they misbehave.

Task.

The goal was to make a problem that could take in two numbers, and construct n distinct multiplication problems where the user is asked to answer what x * y is.

Challenge.

Where I struggled was with the part in cursive. How do I write a clear and readable code that produces distinct multiplication problems? I tried to do it with creating a sample of distinct numbers X, and then choose y such that y ≤ x where x is a member of X. Unsure if this is the best idea.

The code and some comments

I choose to do import random instead of importing the modules as it is a bit clearer where the modules comes from
As above, choose to include a short ___docstring___ more out of habit than necessity I guess..
As mentioned above, do not really like my method for forcing the questions to be distinct.
Tried to comply with pep8 for the long strings, looks a bit strange.

Here is the code

import random

QUESTIONS = 5
MIN_VAL = 2
MAX_VAL = 12

def arithmetic_game(val_1=MIN_VAL,
                    val_2=MAX_VAL,
                    num_of_problem=QUESTIONS):
    """ Ask the user a series of arithmetic questions
    and returns the number of correct answers.

    Args: val_1 (int): Defines the range of multiplication
    Args: val_2 (int): Defines the range of multiplication
    Args: number_of_problems (int): Defines the number of questions

    Return: correct_answers (int): number of correct math questions answered"""

    correct_answers = 0

    num_lst = random.sample(
            range(min(val_1, val_2), max(val_2, val_2)+1),
            num_of_problem)

    for num_1 in num_lst:
        num_2 = random.randint(2, num_1)
        print('\nWhat is {:d} times {:d}?'
              ''.format(num_1, num_2))
        answer = num_1*num_2

        try:
            if int(input()) == answer:
                print('Yes, the answer is {:d}'
                      ''.format(answer))
                correct_answers += 1
            else:
                print('No, the correct answer was {:d}'
                      ''.format(answer))
        except NameError:
            raise ValueError('I am sorry, you have'
                             'to write in a valid integer')

    return correct_answers

if __name__ == "__main__":

    correct_answers = arithmetic_game()
    print('\nYou got {:d} out of {:d} answers correct'
          ''.format(correct_answers, QUESTIONS))

I think you meant to write max(val_1, val_2) instead of max(val_2, val_2).

Long Strings

I personally think the PEP8's 79 characters-per-line requirement is too strict and is not practically optimal. Increase it to, for instance, 100 for this code snippet and you'll get a more readable and concise code with all of the used strings fitting on a single line.

Other Improvements

I would apply an "Extract Variable" refactoring method and define a tuple of report template strings, which would allow to bring the nestedness 1 level down:

is_answer_correct = int(input()) == answer
print(ANSWER_TEMPLATES[is_answer_correct].format(answer))
correct_answers += is_answer_correct

where ANSWER_TEMPLATES is defined as:

ANSWER_TEMPLATES = (
    'No, the correct answer was {:d}',
    'Yes, the answer is {:d}'
)

As far as generating unique random pairs for the problem, there is also this approach involving keeping track of seen pairs and keeping generating a next pair until it has not been seen.


Insertion sort algorithm in Java I have tested this code and it yields the expected outputs, but I want to verify if I have applied insertion sort algorithm correctly or not.

 import java.util.*;
public class InsertionSort
{
    static void swap(int[] arr, int i, int j)
    {
        int temp = arr[i];
        arr[i] = arr[j];
        arr[j] = temp;
    }

    static void sorter(int arr[])
    {

        int hole;
        int n = arr.length;
        if(n>=2)
        {
            for(int i = 1; i<n ; i++)
            {
                hole = i;
                inner:
                for(int j = i-1; j>=0; j--)
                {
                    if(arr[j]> arr[hole])
                    {
                        swap(arr, j, hole);
                        hole = j;
                    }
                    else
                        break inner;
                }
            }
        }
        System.out.println(Arrays.toString(arr));
    }
}

Labeled loops

You say

                        break inner;

But that's not necessary.  The inner loop is already the innermost loop.  You could just say break;.

And frankly, you could have just said

                for (int j = i-1; j >= 0 && arr[j] > arr[hole]; j--)

No break or internal if needed.

But that's not the question you really wanted answered.

Insertion sort?

It's similar to an insertion sort.  Your insertion method is a bit odd though.  Normally insertion sort would find the insertion point and then move the elements after that.  You're bubbling the current value forward.

Consider

int j = i - 1;
while (j >= 0 && numbers[j] < numbers[i]) {
    j--;
}

int temp = numbers[i];
for (int k = i; k > j; k--) {
    numbers[k] = numbers[k - 1];
}

numbers[j] = temp;

That does about half as many array assignments as your method.

Naming

I prefer a descriptive name like numbers to a generic abbreviation like arr.  At minimum, it should be array.  But I prefer numbers.

    static void sorter(int arr[])

As a general rule, we name classes and objects with noun names.  Methods get verb names.  So

    static void sort(int[] numbers) {

In Java, we also normally write arrays as int[] name not int name[].

You could name the class

class InsertionSorter {

Although in this case InsertionSort can be used as a noun.

Wildcard imports

import java.util.*;

The general policy is to import one class at a time, not a wildcard group.  That's especially true here, since it's not clear that you are using any java.util classes.

Mixing logic and display

It's generally agreed that it is better to return the data and display it separately.  Since you are sorting in place, that could just look like

InsertionSort.sort(numbers);
System.out.println(Arrays.toString(numbers));

Declare at initialization

You have

                hole = i;

You could do

                int hole = i;

It's generally considered best practice to declare variables as late as possible.  You don't use hole outside the loop or across iterations, so there's no need to declare it outside the loop.

Redundant logic

        if(n>=2)
        {
            for(int i = 1; i<n ; i++)

If n is 1 or less then 1 < n will be false and it won't enter the loop.

And you don't need n.  You can just say numbers.length whenever you need that value.

Standard methods

You could also replace these two loops with Java standard methods.

        int insertionPoint = Arrays.binarySearch(numbers, 0, i, numbers[i]);
        if (insertionPoint < 0) {
            insertionPoint = -insertionPoint - 1;
        }

        int temp = numbers[i];
        System.arraycopy(numbers, insertionPoint, numbers, insertionPoint + 1, i - insertionPoint);
        numbers[insertionPoint] = temp;

Of course, it's possible that you were trying to avoid standard methods.  After all, Arrays.sort would solve this problem without additional coding.


Ledger query using window functions This is a ledger kind of table, initial amount is when the movement is null and all transactions on it is based on the Waste_Seq

the query take the last entry (id) based on the movement and change the initial amount

this query seem to work for my scenario and seem fast, is there a better or alternative way of doing what the query does?

long term this table will have more than 5-10 millions rows easily, this is a new empty table for now

Schema and data:

CREATE TABLE [dbo].[fredou](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [Movement] [int] NULL,
    [Waste_Seq] [int] NOT NULL,
    [Amount] [int] NOT NULL,
    [operation] [int] NOT NULL,
 CONSTRAINT [PK_fun_fred_test] PRIMARY KEY CLUSTERED
(
    [Id] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

CREATE NONCLUSTERED INDEX [NonClusteredIndex] ON [dbo].[fredou]
(
    [Waste_Seq] ASC,
    [Movement] ASC,
    [Id] DESC
)
INCLUDE (   [Amount],
    [operation]) WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]

insert into fredou values (         null,         1,      1000,         1)
insert into fredou values (         1,         1,       500,        -1)
insert into fredou values (         1,         1,       450,        -1)
insert into fredou values (         1,         1,       475,        -1)
insert into fredou values (         1,         1,       475,        -1)
insert into fredou values (         2,         1,       300,         1)
insert into fredou values (         2,         1,       200,         1)
insert into fredou values (         2,         1,       100,         1)
insert into fredou values (         3,         1,       300,         0)
insert into fredou values (         3,         1,       350,         0)
insert into fredou values (         4,         1,        50,        -1)
insert into fredou values (         4,         1,        50,         0)
insert into fredou values (         null,         2,      5000,         1)
insert into fredou values (        50,         2,      1000,        -1)
insert into fredou values (        50,         2,      1000,         0)
insert into fredou values (        51,         2,       500,        -1)
insert into fredou values (        53,         2,       400,        -1)
insert into fredou values (        51,         2,       300,        -1);

Query:

DECLARE @SEQ int = 1

SELECT SUM(f.Amount * f.Operation) as Amount
FROM
(
    SELECT  FIRST_VALUE(a.Amount)       OVER (partition BY a.Movement order by a.id desc) as Amount,
            FIRST_VALUE(a.Operation)    OVER (partition BY a.Movement order by a.id desc) as Operation,
            ROW_NUMBER()                OVER (partition BY a.Movement order by a.id desc) as rownum
    FROM [dbo].[fredou] a
    WHERE a.Waste_Seq = @SEQ
) f
where f.rownum = 1;

Link to sql fiddle

I doubt FIRST_VALUE is necessary since you have where f.rownum = 1;


Reads paragraphs of common artists separated by commas I want to make this faster, I'm not a great coder and just want to learn. This program reads paragraphs of common artists separated by commas. The user inputs the name of a desired artist and the program searches through the paragraphs to find all related artists and sorts them into order of most common to least common. The artist text file is very large 600kb of text so perhaps that's the reason it's slow.

class ArtistSuggester:
    fileName = "list_of_artists.txt"
    arrayOfLists = []
    artist = input("Enter Artist Name: ")

    @classmethod
    def make_array(cls):
        # sets text to 2D-array
        f = open(cls.fileName, 'r', encoding="utf8")
        curr_par = f.read().split("\n")
        for line in curr_par:
            cls.arrayOfLists.append(line.split(","))

    @classmethod
    def get_locations(cls):
        # searches array for given artist and returns array with areas where the artist was found
        locations = []
        for x in range(0, len(cls.arrayOfLists)):
            for y in range(0, len(cls.arrayOfLists[x])):
                if cls.arrayOfLists[x][y] == cls.artist:
                    locations.append(x)
        return locations

    @staticmethod
    def search_array_for_artist(the_list, artist):
        # searches given array for the given artist's name and returns the position
        # if found nothing returns negative number to symbolize non-existence
        pos = 0
        for x in range(0, len(the_list)):
            if the_list[x] == artist:
                pos = x
                break
            else:
                pos = -1
        return pos

    @classmethod
    def ordered_suggestion_list(cls):
        # makes the final suggestion list in correct order
        # makes two arrays, one with the names and the other with the counts of each throughout the text file.
        check = cls.get_locations()
        final_list = cls.arrayOfLists[check[0]]
        final_list.remove(cls.artist)
        count = [1] * int(len(final_list))
        for x in range(1, len(check)):
            for y in range(0, len(cls.arrayOfLists[check[x]])):
                if cls.arrayOfLists[check[x]][y] == cls.artist:
                    continue
                elif cls.search_array_for_artist(final_list, cls.arrayOfLists[check[x]][y]) > 0:
                    count[cls.search_array_for_artist(final_list, cls.arrayOfLists[check[x]][y])] += 1
                else:
                    final_list.append(cls.arrayOfLists[check[x]][y])
                    count.append(1)
        # makes a dict based off the count and names list to combine values to the keys
        combined_list = dict(zip(final_list, count))
        new_list = []
        for key, value in sorted(combined_list.items(), key=lambda x: x[1], reverse=True):
            new_list.append(key)
        print(new_list)

Does this really have to be a class?
You have alot of classmethod's making me wonder if this really has to be a class.
Review

Change the datatype of Artists
You can change it to a set making the lookup a O(0)instead of O(n) operation

You never close the file
It is better to use with open(a) as a: context so python will automatically close the file

We can use collections.Counter to count the elements

Code
from collections import Counter

ARTISTS_FILE = 'list_of_artists.txt'

def read_file():
    with open(ARTISTS_FILE, "r") as f:
        return [
                    set(artist.strip().split(','))
                    for artist in f.readlines()
                ]

def make_ordered_list(artist):
    artists = read_file()
    counts = Counter()
    for similar in artists:
        if artist in similar:
            similar.remove(artist)
            counts.update(similar)
    return sorted(counts, key=lambda x: (-counts[x], x))

def main():
    artist = input("Type the artist: ")
    print(make_ordered_list(artist))

if __name__ == '__main__':
    main()


Simulates flipping a coin repeatedly and continues until however many consecutive heads are tossed This program simulates flipping a coin repeatedly and continues until however many consecutive heads are tossed.

I'm wondering if there are any issues when initializing a variable in a for loop the way I did. Also I assume assigning -1 to i was an appropriate move as well because after a loop cycle it will iterate (i++) causing i to become 1 if I had not, resulting in one less loop.

/*
 * File: ConsecutiveHeads.java
 * ---------------------------
 * This program simulates flipping a coin
 * repeatedly and continues until however many
 * consecutive heads are tossed.
 */

/* Library packages */
import acm.program.*;
import acm.util.RandomGenerator;

public class ConsecutiveHeads extends ConsoleProgram {

    public void run() {
        int flips = 0;
        for (int i = 0; i < CONSECUTIVE_HEADS; i++) {
            String coinSide = flipCoin();
            println(coinSide);
            flips++;
            if (coinSide.equals("Tails")) {
                i = -1;
            }
        }
        println("It took " + flips + " flips to "
                + "get " + CONSECUTIVE_HEADS + " consecutive heads.");
    }

    /* Returns true 50% of the time */
    private String flipCoin() {
        return rgen.nextBoolean() ? "Tails" : "Heads";
    }

    /* Create an instance variable for the random number generator */
    private RandomGenerator rgen = RandomGenerator.getInstance();

    /* Private constant */
    private static final int CONSECUTIVE_HEADS = 3;
}

Loop

  Also i assume assigning -1 to i was an appropriate move as well because after a loop cycle it will iterate (i++) causing i to become 1 if I had not, resulting in one less loop.

Well, that works, but I think it would be more clear to explicitly increment i on heads, and reset it otherwise:

for (int consecutiveHeads = 0; consecutiveHeads < CONSECUTIVE_HEADS_THRESHOLD;) {
  ...
  if (coinSide.equals("Heads")) {
    consecutiveHeads++;
  } else {
    consecutiveHeads = 0;
  }
}

That could be a while loop as well, depending on your tastes.

Comments

/* Create an instance variable for the random number generator */

This is not really necessary. It's clearly an instance variable, and the names and types show that it is a RNG.

/* Private constant */

Again, this isn't really necessary. private final already shows that is is a private constant. Even if you these comments are more intended for organization, a common approach would be to group these variables in blocks at the top of the class, with implicit meaning.

They don't take away from anything, they're just not really necessary.

/* Returns true 50% of the time */
private String flipCoin() {

This is not accurate. It will never return true. It returns strings.

Matching strings

Matching against strings is somewhat fragile. If this was a larger codebase, you would probably implement an enum for the coin sides, although it's overkill for this example. Altogether, a slightly more genericized version might look something like this:

public class ConsecutiveHeads extends ConsoleProgram {

  // These traditionally go up top, not at the bottom of the class.
  private final RandomGenerator rgen = RandomGenerator.getInstance();
  private static final int DEFAULT_CONSECUTIVE_HEADS_THRESHOLD = 3;

  public void run() {
    run(DEFAULT_CONSECUTIVE_HEADS_THRESHOLD);
  }

  // Theshold is parameterized
  public void run(final int consecutiveHeadsThreshold) {
    int flips = 0;
    for (int consecutiveHeads = 0; consecutiveHeads < consecutiveHeadsThreshold;) {
      CoinSide toss = flipCoin();
      flips++;
      println(toss.getName());

      if (CoinSide.HEADS.equals(toss)) {
        consecutiveHeads++;
      } else {
        consecutiveHeads = 0;
      }
    }
    System.out.printf("It took %d flips to get %d consecutive heads.", flips, consecutiveHeadsThreshold);
  }

  private CoinSide flipCoin() {
    return CoinSide.from(rgen.nextBoolean());
  }

  // Alternatively, in the limited scope of this example, you could
  // just define `true` to be heads and then not bother defining tails
  // at all.
  enum CoinSide {
    HEADS("Heads"), TAILS("Tails");

    private final String name;
    CoinSide(String name) { this.name = name; }
    public String getName() { return name; }

    public static CoinSide from(bool value) { return value ? HEADS : TAILS; }
  }
}


C++ environment variable expansion I wrote the following a while ago (using regex code I found on Stack Overflow), and I have the following concern I'd like addressed: should there be an std::move() around the passing of the string into the parameter in the second version of the function, ExpandEnv(std::string const &text), to the rvalue version? Conversely, is there any reason not to use it?

inline std::string GetEnv(char const var[])
{
#if defined _WIN32 // getenv() is deprecated on Windows
    char *buf{nullptr};
    size_t sz;
    std::string val;
    if (_dupenv_s(&buf, &sz, var) || buf == nullptr) return val;
    val = buf;
    free(buf);
    return val;
#else
    return getenv(var);
#endif
}

inline std::string ExpandEnv(std::string &&text)
{
    static const std::regex ENV{"\\$\\{([^}]+)\\}"};
    std::smatch match;
    while (std::regex_search(text, match, ENV)) text.replace(match.begin()->first, match[0].second, GetEnv(match[1].str().c_str()));
    return text;
}

inline std::string ExpandEnv(std::string const &text)
{
    return ExpandEnv(std::string(text));
}

You don't need to overload ExpandEnv.  In the const-ref overload, your action is to make a copy of the argument and pass that to the rvalue-ref version.
There's a simple way to get the compiler to do that, and it's pass by value.  Often overlooked, this is exactly the right way to get a modifiable value in your function, by move if possible, and by value if not:
inline std::string ExpandEnv(std::string text)
{
    //...implementation
}

To answer your specific question (even though we've eliminated the relevant code), there's no need for std::move(), as std::string(...) is already an rvalue expression.

Other notes:

Your local style guide may say otherwise, but we normally use PascalCase for names of composite types rather than for functions.  Consistency with the rest of the code base and any libraries may be a factor here, though.

That Windows code instead of getenv() is horrible - I hope I never have to target Windows!  Can _dupenv_s() return true when it has allocated buf?  If so, the error path also needs to free(buf).

buf == nullptr can be written simply as !buf.

In the Windows path, you can avoid the need for local val by giving buf to a unique_ptr to look after it for you (so you can simply return buf; or return {}; for the success and failure cases, respectively):
  char *buf{nullptr};
  size_t sz;
  auto failed = _dupenv_s(&buf, &sz, var);
  std::unique_ptr<char,malloc_deleter> m{buf}; // ensure buf is released automatically
  return failed ? std::string{} : std::string{buf, sz};

Actually, now I've read up on the Windows function, there seems to be no reason not to use std::getenv() - the only difference in behaviour seems to be if a null pointer is passed as argument, and we never do that.

You might want to use a raw string literal to reduce those doubled-backslashes: \$\{([^}]+)\} may be slightly easier to read.

Really, really split that single-line while into separate lines for the condition and statement.  It may even be worth making it a block, and giving names to the match and replacement - notably for match[0] which you also write as match.begin() in that expression.

I'd probably write the argument to GetEnv() as char const* rather than char const[], to help distinguish from actual array arguments (in templates, where the size is a template argument).

Do you want to provide a means to escape ${ when it appears literally in the input?

#Reworked code
#include <string>
#include <regex>
#include <cstdlib>

inline std::string expand_env(std::string text)
{
    static const std::regex env_re{R"--(\$\{([^}]+)\})--"};
    std::smatch match;
    while (std::regex_search(text, match, env_re)) {
        auto const from = match[0];
        auto const var_name = match[1].str().c_str();
        text.replace(from.first, from.second, std::getenv(var_name));
    }
    return text;
}

#include <iostream>
int main()
{
    std::cout << expand_env("${HOME} is where the heart is.\n");
}


Library system manager I'm new to JavaScript OOP and still playing around with it. I have put together a few specific questions over specific details throughout the code that are bothering me.

I wanted my Library class constructor to be optional, the option to add a single book, multiple books or none at all, when creating the  object. Is this a good way of achieving this goal?

class Library{
  constructor(items){
    this.books = [];
    if(typeof items != 'undefined' && items instanceof Array){
      this.books.push.apply(this.books, items); // Concat two arrays
    } else if(typeof items != 'undefined') {
      this.books.push(items); // Add one element
    }
  }

  checkIn(id, renter){
    var index = this.getIndex(id);
    if(!this.books[index].available){
      this.books[index].setDefaults();
      renter.removeBook(id);
    } else {
      throw "Book Already Checked In";
    }
  }

  checkOut(id, renter){
    var index = this.getIndex(id);
    if(this.books[index].available){
      this.books[index].available = false;
      this.books[index].currentRenter = renter.name;

      // Adds current date to check out information.
      var dateFormat = require('dateformat');
      this.books[index].checkOutDate = dateFormat(new Date(),  "dddd, mmmm dS, yyyy, h:MM:ss TT");

      renter.addBook(this.books[index]); // Adds book to renter's inventory.
    } else {
      throw "Book Not Available";
    }
  }

  getIndex(id){
    for(var i = 0; i < this.books.length; i++){
      if(this.books[i].id == id) return i;
    }
    return -1;
  }
}

My Book class works, but just does not feel correct at all and looks very sloppy. Any thoughts?

class Book{
  constructor(title, pageCount, id){
    this.title = title;
    this.pageCount = pageCount;
    this.id = id;
    this.available = true;
    this.currentRenter = null;
    this.checkOutDate = null;
  }

  setDefaults(){
    this.available = true;
    this.currentRenter = null;
    this.checkOutDate = null;
  }
}

These are the only classes I did not really have any questions over. I am aware splitting these classes into two is completely unnecessary in this scenario, but I was just messing around with inheritance and never changed it back.

class Person{
  constructor(name, age, phoneNumber){
    this.name = name;
    this.age = age;
    this.phoneNumber = phoneNumber;
  }
}

class Renter extends Person{
  constructor(name, age, phoneNumber, libraryID){
    super(name, age, phoneNumber)
    this.libraryID = libraryID;
    this.books = [];
  }

  addBook(book){
    this.books.push(book);
  }

  removeBook(id){
    var index = this.getIndex(id);
    this.books.splice(index, 1);
  }

  getIndex(id){
    for(var i = 0; i < this.books.length; i++){
      if(this.books[i].id == id) return i;
    }
    return -1;
  }
}

Another general question: Am I accessing the class variables "formally"? It does not feel right setting a instance variable without a setter or returning a instance variable without a getter.

checkIn(id, renter){...}
checkOut(id, renter){...}

You're not actually checking in the id of a book. You're checking in a book. So I would suggest accepting an instance of a book and get the ID from it instead of passing in an ID. You're already doing it for renter, passing in the renter instance, so it shouldn't be different.

this.books = [];

I would suggest using an object {} instead of an array. One problem with using an array, and the ID as index is that you unknowingly increase the length of the array. If you check the length of an empty array after doing this.books[100] = new Book(...), you'll see that it's 101. The array stretched to 101 elements and left gaps in between.

If the book is checked out, you can either null its entry in the object or delete it altogether with delete. It also makes getIndex unnecessary as you can simply access the object directly with id as key.

throw "Book Already Checked In";

I would suggest throwing an instance of Error instead of a string. Error-catching code will most likely expect an instance of Error and not a string. You also lose out on extra information that comes with an instance of Error. Lastly, not OOP-ish. :P

checkOut(id, renter){
  ...
  var dateFormat = require('dateformat');

Dependencies should be done outside anything. Behavior depends on the loader used. If you used RequireJS this would try to resolve dateformat, and load it on the fly. Other module loaders may inline the code, causing this function to bloat.

this.currentRenter = null;
this.checkOutDate = null;

The book shouldn't be aware of its renter. All it should be describing is the book itself.

If you want something like a library card, then I suggest making a property libraryCard that holds an array of LibraryCardRecord objects, each containing the date of checkout and the id of the renter at that date.

  It does not feel right setting a instance variable without a setter or returning a instance variable without a getter.

It's overkill, but it's a good practice to always use a getter and setter. While it's meaningless in the world of all-public JavaScript, it allows you to do intermediate operations before setting or getting. ES6 does have special syntax for getters and setters.


Setting locale based on preferences with fallbacks Consider the following code. Are the multiple assignments to the loc variable a code smell? If so, how can I refactor this?

def set_locale
  begin
    loc = user_signed_in? ? user_locale : browser_locale
    loc = browser_locale if loc.nil?
    loc = param_locale if loc.nil?
    loc = I18n.available_locales.include?(loc.to_sym) ? loc.to_sym : I18n.default_locale
  rescue
    loc = I18n.default_locale
  ensure
    cookies.permanent[:my_locale] = loc
  end
  I18n.locale = loc
end

I'd say it's a code smell. Mostly because loc seems to change type. It's one thing for it to be nil or a symbol; in that case it might "only" change value. But the last to_sym would seem to suggest it could also be a string or something.

In terms of code style, you could just use the ||= assignment operator instead of x = y if x.nil?. That's the Ruby way to provide a fallback/default value in case of nil.

However, you're looking to prioritize locale selection, in which case I'd do something like:

locale = [user_locale, browser_locale, param_locale]
  .compact
  .map(&:to_sym)
  .detect { |locale| I18n.available_locales.include?(locale) }

I18n.locale = locale || I18n.default_local
cookies.permanent[:my_locale] = I18n.locale

It'll go through the list and return the first locale that's not nil and is available. If nothing's returned, it uses the default locale.

Note that I'm making an assumption here, namely that user_locale will return nil if the user isn't signed in. But you could also simply unshift the user_locale onto the array if the user is signed in, before doing the detect.

On another note: param_locale should, I think, be the first choice in all cases. If I go to a URL that has an explicit locale code in it, I'd expect to see exactly that page, in that language. URLs should be a permanent as you can make them. Sure, give me an option to view the same URL with a different locale, but don't second guess the URL itself.


Calculate CPU% by process from /proc/stat and /proc//stat We don't have the possibility to install modules like psutil on some boxes, thus I decided to write a simple script to calculate the CPU percentage by the given PID.

It uses /proc/stat to get CPU times and /proc/<pid>/stat to get the process CPU usage.

I added top output here, just to compare results and to be sure it's (about) correct. But - I don't like to use external utilities in Python scripts, thus top will be removed.

OS: CentOS 6.5
Python: 2.6.6

Next - it calculates % of CPU, taken by this process:

#!/usr/bin/env python

import sys
import os
import time
import subprocess

if len(sys.argv) == 2:
    pid = sys.argv[1]
else:
    print('No PID specified. Usage: %s <PID>' % os.path.basename(__file__))
    sys.exit(1)

def proct(pid):
    try:
        with open(os.path.join('/proc/', pid, 'stat'), 'r') as pidfile:
            proctimes = pidfile.readline()
            # get utime from /proc/<pid>/stat, 14 item
            utime = proctimes.split(' ')[13]
            # get stime from proc/<pid>/stat, 15 item
            stime = proctimes.split(' ')[14]
            # count total process used time
            proctotal = int(utime) + int(stime)
            return(float(proctotal))
    except IOError as e:
        print('ERROR: %s' % e)
        sys.exit(2)

def cput():
    try:
        with open('/proc/stat', 'r') as procfile:
            cputimes = procfile.readline()
            cputotal = 0
            # count from /proc/stat: user, nice, system, idle, iowait, irc, softirq, steal, guest
            for i in cputimes.split(' ')[2:]:
                i = int(i)
                cputotal = (cputotal + i)
            return(float(cputotal))
    except IOError as e:
        print('ERROR: %s' % e)
        sys.exit(3)

# assign start values before loop them
proctotal = proct(pid)
cputotal = cput()

try:
    while True:

        # for test, to compare results
        proc = subprocess.Popen("top -p %s -b -n 1 | grep -w mysql | awk '{print $9}'" % pid, shell=True, stdout=subprocess.PIPE)
        cpu_percentage = proc.communicate()
        print('With TOP: %s' % (cpu_percentage[0].rstrip('\n')))

        pr_proctotal = proctotal
        pr_cputotal = cputotal

        proctotal = proct(pid)
        cputotal = cput()

        try:
            res = ((proctotal - pr_proctotal) / (cputotal - pr_cputotal) * 100)
            print('With script: %s\n' % round(res, 1))
        except ZeroDivisionError:
            pass

        time.sleep(1)
except KeyboardInterrupt:
    sys.exit(0)

And its results:

$ ./cpu_usage_by_proccess.py 24846
With TOP: 0.0
With script: 0.0

With TOP: 0.0
With script: 0.0

With TOP: 0.0
With script: 0.7

With TOP: 14.0
With script: 4.8

With TOP: 32.0
With script: 34.4

With TOP: 29.9
With script: 32.6

With TOP: 30.0
With script: 35.1

With TOP: 18.0
With script: 26.5

With TOP: 20.0
With script: 20.6

What's done incorrectly here, or what can be done better (maybe a more "Pythonic way")?

You have code outside of functions or an if __name__ == "__main__" guard (which isn't ideal in itself), both above and below the various function definitions, which makes the code relatively difficult to follow. I would structure the script as follows:

import os
import subprocess
import sys
import time

def proct(pid):
    ...

def cput():
    ...

def main(pid):
    ...

def parse_args():
    ...
    return pid

if __name__ == "__main__":
    main(parse_args())

Now if you want to import this functionality elsewhere, it's easy. Also, note that I have made the imports alphabetical, per the style guide.

I would factor out any temporary variables, e.g.

proctotal = int(utime) + int(stime)
return(float(proctotal))

can be written:

return float(int(utime) + int(stime))

Try not to do the same thing twice, e.g.

proctimes = pidfile.readline()
utime = proctimes.split(' ')[13]
stime = proctimes.split(' ')[14]

could be rewritten

proctimes = pidfile.readline().split(' ')
utime = proctime[13] # you could also call int here
stime = proctime[14]

which only calls str.split once.

Consider adding docstrings to your functions, rather than the inline comments (some of which are redundant - utime = proctimes.split(' ')[13] does not need the explanation # get utime from /proc/<pid>/stat, 14 item, which just adds an extra line to read). Better function names would also help with clarity - e.g. cpu_total rather than cput.

The main part of cput:

cputimes = procfile.readline()
cputotal = 0
for i in cputimes.split(' ')[2:]:
    i = int(i)
    cputotal = (cputotal + i)
return(float(cputotal))

can be rewritten in a functional style:

return sum(map(float, procfile.readline().split(' ')[2:]))

or using a generator expression:

return sum(float(s) for s in procfile.readline().split(' ')[2:])


Compare 2 Different database Table I have php function that compares two different database table. Any suggestion on improving it.

     public function comparePartyTableCandidates()
   {
      $electionCandidates =  ElectionCandidate::all();
      foreach($electionCandidates as $electionCandidate )
      {
         $candidateName = $electionCandidate->CandidateFirstname. " ".$electionCandidate->CandidateLastname ;

         $result = Candidate::where('candidateName' ,'=',$candidateName )->first();

         if(empty($result))
        {

           $electionconstituency= ElectionConstituency::where('ConstituencyID', '=',$electionCandidate->ConstituencyID)->first();
           $constituency = Constituency::where('ConstituencyName', '=',strtolower($electionconstituency->ConstituencyName))->first();
            echo $electionconstituency->ConstituencyName;
            if(empty($constituency))
            {
                if((preg_match("/KINGSTON EAST & PORT ROYAL/",$electionconstituency->ConstituencyName)))
                {
                    $constituency = Constituency::where('ConstituencyName', 'like',"%"."kingston Eastern")->first();
                }
            }

           $electionparty = ElectionParty::where('PartyID', 'like', '%' . $electionCandidate->PartyID)->first();
           $politicalParty = PoliticalParty::where('partyName', 'like', '%' . $electionparty->PartyAbbr)->first();

           $candidate = new Candidate();
           $candidate->candidateName = $candidateName;
           $candidate->constituency_Id = $constituency->id;
           $candidate->party_id = $politicalParty->id;
           $candidate->save();
         }
      }
   }

TL;DR: Be clear, consistent and efficient, don't assume things and verify that the output of functions is as expected

Your code doesn't just compare candidates, it also creates new records. As such your function should be appropriately named e.g. compareAndCreateCandidates.
Comments are useful. You can declare a phpdoc style comment for your method and add comments to clarify any corner case e.g. the reasoning behind the if (empty($constituency)) code block.
Searches with 'LIKE', '%'. imply that unique identifiers are not being used. The % should be removed if the fields are unique. If not, you should find a unique field or a combination of fields that is unique for each entry and use that instead.
Two candidates in different parties and constituencies cannot share the same name i.e. a John Smith will not be created if another exists in the database. The criteria should be updated as in (3) above.
Since you are using laravel you should stick to the PSR-2 coding conventions e.g. for control structures:

  There MUST be one space after the control structure keyword
  There MUST NOT be a space after the opening parenthesis
  There MUST NOT be a space before the closing parenthesis
  There MUST be one space between the closing parenthesis and the opening brace
  The structure body MUST be indented once
  The closing brace MUST be on the next line after the body

Use strpos() instead of preg_match(). From the docs:

  Do not use preg_match() if you only want to check if one string is contained in another string. Use strpos() instead as it will be faster.

Use either single or double quotes consistently for plain strings:

where('ConstituencyName', 'like',"%"."kingston Eastern")
Since you are using camelCase for variables make sure this is consistent e.g. $electionparty, $electionconstituency
The line echo $electionConstituency->ConstituencyName; looks like test code and should be removed. If not, then it violates the PSR-1 requirement that files should either declare something or have a side effect e.g outputting html. The ConstituencyNames should be saved in an array or string and returned by the function instead of echoing them.
Several variables are only used once and should not be initialized e.g $electionCandidates, $result
There is no check to ensure $electionParty and $politicalParty are not null.
The code assumes each model will be saved without error. The database saves should be wrapped in a transaction with proper error handling.

The code with some issues fixed is as follows:

/**
 * This is where a description of the method's purpose goes
 */

public function compareAndSaveCandidates()
{
    foreach (ElectionCandidate::all() as $electionCandidate) {
        $candidateName = $electionCandidate->CandidateFirstname . " " . $electionCandidate->CandidateLastname;
        if (!Candidate::where([
             ['candidateName', '=', $candidateName],
             //other criteria
        ])->first()) {
            $electionconstituency = ElectionConstituency::where('ConstituencyID', '=', $electionCandidate->ConstituencyID)->first();
            $constituency = Constituency::where('ConstituencyName', '=', strtolower($electionConstituency->ConstituencyName))->first();
            //echo $electionConstituency->ConstituencyName;

            //This is a corner case and deserves a comment
            if (!$constituency) {
                if ((strpos($electionConstituency->ConstituencyName, 'KINGSTON EAST & PORT ROYAL'))) {
                    $constituency = Constituency::where('ConstituencyName', 'like', '%' . 'kingston Eastern')->first();
                }
            }

            $electionParty = ElectionParty::where('PartyID', 'like', '%' . $electionCandidate->PartyID)->first();
            $politicalParty = PoliticalParty::where('partyName', 'like', '%' . $electionParty->PartyAbbr)->first();

            $candidate = new Candidate();
            $candidate->candidateName = $candidateName;
            $candidate->constituency_Id = $constituency->id;
            $candidate->party_id = $politicalParty->id;
            $candidate->save();
        }
    }
}


Apply same format function to each python print() parameter I have a python print statement and inside the string I wish to print are four digits. I would like to apply the same formatting function to each param. I am not familiar with the latest and greatest features of python PEPs.

Is there a slick way to do this?

Code

statement = "Splitting up the {} file into {} chunks, with the filesystem block size of {}, causing {} extra space to be used"

print(statement.format(
    sizeof_fmt(input_file_size),
    sizeof_fmt(chunk_size),
    sizeof_fmt(block_size),
    sizeof_fmt(surrendered_space)))

Format Function

def sizeof_fmt(num, suffix='B'):
    for unit in ['','Ki','Mi','Gi','Ti','Pi','Ei','Zi']:
        if abs(num) < 1024.0:
            return "%3.1f%s%s" % (num, unit, suffix)
        num /= 1024.0
    return "%.1f%s%s" % (num, 'Yi', suffix)

Time will tell if your question is considered a worthy Code Review question, but till then I'ld like you to give a short review on your code nevertheless.

Format function

You could reduce the code duplication in the format function and make use of .format or f-strings (from Python 3.6 onwards).

def sizeof_fmt_rev(num, suffix='B'):
    for unit in ['', 'Ki', 'Mi', 'Gi', 'Ti', 'Pi', 'Ei', 'Zi']:
        if abs(num) < 1024.0:
            break
        num /= 1024.0
    else:
        # this part is only executed if the loop was not left with a break
        unit = 'Yi'
    return f"{num:.1f}{unit}{suffix}"

This uses for ... else, one of the less well-known features of Python and only has a single line where the format expression has to be written. I see a chance to build something using math.log instead of that loop, but I will leave that as an exercise to you. You can even build something that works without a loop, but at least the version I came up with (found below) is actually slower than the original implementation.

def sizeof_fmt_rev_log(num, suffix='B'):
    exponent = min(int(math.log(abs(num), 1024)), 8)
    num /= 1024**exponent
    unit = ('', 'Ki', 'Mi', 'Gi', 'Ti', 'Pi', 'Ei', 'Zi', 'Yi')[exponent]
    return f"{num:.1f}{unit}{suffix}"

I used

for i in range(10):
    num = 3.8 * 1024**i
    print(sizeof_fmt_rev(num))
    assert sizeof_fmt(num) == sizeof_fmt_rev(num)
    assert sizeof_fmt(-num) == sizeof_fmt_rev(-num)

to test the revised version.

Code

As @AJNeufeld mentions in his comment, you could use map to save yourself some typing

print(
    statement.format(*map(sizeof_fmt, (input_file_size, chunk_size, block_size, surrendered_space)))
)

which is functionally equivalent to using a list comprehension:

print(
    statement.format(*[
        sizeof_fmt(i)
        for i in (input_file_size, chunk_size, block_size, surrendered_space)
    ])
)

Both build upon a technique called tuple unpacking, but as you can see it can also be used with lists, other sequences, and maybe also iterables (if it is a generator, it will be consumed - thanks @Graipher, who confirmed it/pointed it out in a comment).


Python program to implement pow(x, n) This is a Leetcode problem:

  Implement pow(x, n), which calculates x raised to the power n (\$x^n\$).

  Note -

  -100.0 < x < 100.0
  n is a 32-bit signed integer, within the range [\$-2^{31}\$, \$2^{31}\$ − 1]

def pow(x, n):
    if n == 0:
        return 1
    if x == 0 and n < 0:
        return
    result = 1
    if n > 0:
        while n > 0:
            pre_result = result
            result *= x
            if pre_result == result or pre_result == -result:
                if result >= 0:
                    return result
                if n % 2 == 0:
                    return -result
                else:
                    return result
            n -= 1
        return result
    else:
        while n < 0:
            pre_result = result
            result /= x
            if pre_result == result or pre_result == -result:
                if result >= 0:
                    return result
                if n % 2 == 0:
                    return -result
                else:
                    return result
            n += 1
        return result

NOTE - My solution is \$O(n)\$.

Here are some example inputs/outputs:

#print(pow(2.00000, 10))

>>> 1024.0

#print(pow(2.10000, 3))

>>> 9.261

#print(pow(2.00000, -2))

>>> 0.25

#print(pow(-1.00000, -2147483648))

>>> 1.0

Here are the times taken for each output:

#%timeit pow(2.00000, 10)

>>> 1.88 µs ± 14 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

#%timeit pow(2.10000, 3)

>>> 805 ns ± 17.2 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

#%timeit pow(2.00000, -2)

>>> 643 ns ± 9.55 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

#%timeit pow(-1.00000, -2147483648)

>>> 594 ns ± 20 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

So, I would like to know whether I could make this program shorter and more efficient.

Any help would be highly appreciated.

Do less writing

You can half the code by using the fact \$x^{-n} = \frac{1}{x^n}\$

if n > 0:
    while n > 0:
    ...
else:
    while n < 0:
    ...

would become

if n < 0:
    # x**(-n) == 1 / x**n
    return 1 / old_code(x, -n)

return old_code(x, n)

(with old code being the code you have from the while loop down)

Likewise \$(-x)^n = x^n\$ if n is even, otherwise \$-(x^n)\$. This extra check can be done at the start rather than in the middle of a loop. Combining the two you get

if n < 0:
    power = -n
else:
    power = n

if x < 0:
    base = -x
else:
    base = x

result = old_code(base, power)

if base != x and power % 2:
    # (-x)**n == -(x**n) if n is odd, otherwise x**n
    result = -result

if power != n:
    # x**(-n) == 1 / x**n
    result = 1 / result

return result

This probably could bit a little nicer, but hopefully it is very clear what is happening. From this you can write your code assuming both the base and power are positive.

Better algorithm

We can improve the algorithm from O(n) to O(log n), where n is the power, using the idea from russian peasant exponentiation. Alternatively you could implement exponentiation by squaring for the same runtime complexity. We can derive the idea by the following observation

$$x^n = \overbrace{x \times x \times x \times \cdots \times x}^{n}$$

$$x^n = \overbrace{x \times x \times x \times \cdots \times x}^{n/2} \times \overbrace{x \times x \times x \times \cdots \times x}^{n/2}$$

$$x^n = y \times y, y = \overbrace{x \times x \times x \times \cdots \times x}^{n/2}$$

We need to make a slight adjustment if n is odd, as n/2 wont be an integer

$$x^n = x \times \overbrace{x \times x \times x \times \cdots \times x}^{\lfloor n/2 \rfloor} \times \overbrace{x \times x \times x \times \cdots \times x}^{\lfloor n/2 \rfloor}$$

$$x^n = x \times y \times y, y = \overbrace{x \times x \times x \times \cdots \times x}^{\lfloor n/2 \rfloor}$$

We can then work out \$x^{n/2}\$ recursively until some easy to compute base case.

Note: To turn this into a loop we can do the same thing, but starting at lower powers and working up. See the pdf for how to continue.

All in all my suggested code would look something like this

def pow(x, n):
    if n == 0:
        return 1
    if x == 1:
        # New easy check
        return 1
    if x == 0:
        if n < 0:
            # builtin pow throws an exception rather than returning None
            raise ZeroDivisionError("...")
        return 0

    if n < 0:
        power = -n
    else:
        power = n

    if x < 0:
        base = -x
    else:
        base = x

    result = _pow(base, power)

    if base != x and power % 2:
        # (-x)**n == -(x**n) if n is odd, otherwise x**n
        result = -result

    if power != n:
        # x**(-n) == 1 / x**n
        result = 1 / result

    return result

def _pow(base, power):
    """Return base**power
    Assume base > 0, power > 0"""
    if power == 1:
        return base

    y = _pow(base, power // 2)
    result = y * y
    if power % 2:
        result *= base
    return result


Handling URL patterns with JS I'm handling parameters that are passed from a URL. I want to keep my URLS neat, and the following (rather exhaustive code) has been able to achieve it.
There will be more added, so it would be good to keep it versatile and compact. Code below does work.
URLS patterns
a.com/

a.com/q=bar

a.com/filters=foo

a.com/q=bar&filters=foo

Current code to account for above
const isSearch = this.props.location.pathname.includes("q=")
const isFilter = this.props.location.pathname.includes("filters=")

const encodedVal = encodeURIComponent(e.target.value)

if (encodedVal && isSearch && isFilter) {
    const filters = this.props.match.params.q.split("filters=")[1].split("&")[0];
    this.props.history.push("/q=" + encodedVal + "&filters=" + filters );
  }
 else if (encodedVal && isFilter) {
    const filters = this.props.match.params.q.split("filters=")[1].split("&")[0];
    this.props.history.push("/q=" + encodedVal + "&filters=" + filters );
  }
 else if (encodedVal && isSearch) {
    this.props.history.push("/q=" + encodedVal);
  }
 else if (isFilter && !encodedVal) {
    const filters = this.props.match.params.q.split("filters=")[1].split("&")[0];
    this.props.history.push("/filters="  + filters);
  }
 else if (encodedVal) {
    this.props.history.push("/q=" + encodedVal);
  }
  else {
    this.props.history.push("/");
  }

Fragile parsing of query parameters

For example, the implementation will silently convert a url like /bbq=grill&pagefilter=foo into /q=grill&filter=foo.

Inappropriate use of query parameters

The example urls use some sort of illegal form of query parameters.
Query parameters should come after ? in a url.
(The part of a url after ? is called the query string.)

Unclear purpose

It seems that encodedVal is the encoded value of q=.
If that's the case, it's not clear from the posted code.
If that's not the case,
then the posted code ignored the value of q=,
and it's not clear why.

On the other hand, the value of filter= is left untouched.

Why not encode all the query values?
It's puzzling.

Complicated parsing of query parameters

The chain of conditions is complicated,
fragile,
and it won't scale if you need to add something else later.
It's hard to understand and to modify.

It would be better to properly tokenize the query parameters into key-value pairs (so an object in JavaScript).
Let's call this object query.
Then, the implementation to build the url fragment can be written a simple straightforward way that's easy to understand and to extend.

var encodedUrl = "";

if (query.q) {
    encodedUrl += "&q=" + encodeURIComponent(query.q);
}

if (query.filters) {
    encodedUrl += "&filters=" + encodeURIComponent(query.filters);
}

// chop off any excess leading "&"
if (encodedUrl) {
    encodedUrl.substr(1);
}

// let's make this a proper query string
this.props.history.push("?" + encodedUrl);


Check user changed his password recently or not in specific period I am working on Oracle SQL. What I am trying to do is:

Query a record with ID
If the record does not exists use user input value
If the record exists, compare last_login_date column and user input value (I put to_date('20160401', 'yyyymmdd') then pick recent one)
Add not_change_day to result of 3, then compare the value with sysdate
If 4 is bigger then sysdate then returns true else false

select
    case when (a.ncd + a.lld > sysdate) then 'true'
    else 'false' end as result
from
(
    select
        nvl(not_change_day, 90) as ncd,
        case
            when last_login_date is null then to_date('20160401','yyyymmdd')
            when last_login_date > to_date('20160401','yyyymmdd') then last_login_date
        END as lld
    from
        FT_PWD_CHANGE
    where
        id='some_id'
) a

RESULT

result
------
false

However, I believe there are too many case when statements and sub-queries. Would you suggest a more efficient way?

Your query doesn't returns a result when there's no id matching 'some_id'.

Well, there's a trick to force a row, use an aggregate function.

I think this is your logic rewritten to a single CASE (of course COALESCE is just a shortcut for a CASE :-)

SELECT COALESCE(MIN(CASE WHEN Greatest(COALESCE(last_login_date, to_date('20160401','yyyymmdd'))
                                      ,to_date('20160401','yyyymmdd')
                             + COALESCE(not_change_day, 90) > sysdate
                         THEN 'true'
                     END)
                    ,'false')
FROM FT_PWD_CHANGE
WHERE id='foo'

Btw, for a hard-coded date I would prefer DATE '2016-04-01' over to_date('20160401','yyyymmdd'), it's shorter and Standard SQL.


Improving the performance of my "type" function I'm writing a JavaScript library, and I am using this function for type detection:

function type(obj){
    return  Object.prototype.toString.call(obj).replace(/([\[\]]|object|\s)/gi, "");
}

Their works ok, but compared to jQuery and MooTools it stinks.

http://jsperf.com/code-type-test-a-test

How can I improve its performance?

Interesting question, since there is only 1 line to review and a limited amount of ways to get the 'type` of an object

I will say that

function type(obj){
  return  Object.prototype.toString.call(obj).replace(/([\[\]]|object|\s)/gi, "");
}
var o = new (function Name(){})();
console.log( type(o) );

returns ""

whereas

function type(o){
  return  o !== undefined && o.constructor ? o.constructor.name : "Undefined";
}
var o = new (function Name(){})();
console.log( type(o) );

returns "Name" which is more correct.

It seems ( though I fudged it a little, a re-test is required ) that the second approach can be faster on IE and Safari ). All in all, I would copy the jQuery source, it takes care of all corner cases and is sufficiently fast.


Transforming an array in Ruby I watched a presentation by Dave Thomas on Elixir where he gave an example problem that he solved using functional programming. He mentioned using Ruby to solve the same problem, but did not show an example. I decided to give it a try. The problem goes something like this:

For a list (Ruby doesn't have lists, but for our purposes an Array is close enough) of n numbers, create a new list where unique numbers are represented once, and repeated numbers are represented by a tuple (again, Ruby doesn't have tuples, but for our purposes a Hash or an Array would do) where the first element is the number itself, and the second is its count.

For Example, the following list:

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

Would become:

[ 1, {2, 3}, 3, {4, 2}, 5, {6, 3} ]

I came up with the following solution.

list = [ 1, 2, 2, 2, 3, 4, 4, 5, 6, 6, 6, 6 ]
list.each_with_object(Hash.new(0)) { |n, hash| hash[n] += 1 }.map { |k, v| v > 1 ? { k => v } : k }
#=> [ 1, {2 => 3}, 3, {4 => 2}, 5, {6 => 3} ]

How would you go about solving this problem?

Some notes:

From the moment you write hash[n] += 1, your solution is not functional anymore.
each_with_object is also imperative, for FP you should use reduce.
reduce works well with linked lists, not so well for generating new arrays. In any case, reduce is a generic abstraction, there is a more specific one to the problem at hand: Enumerable#chunk.

That's how I'd write it:

xs.chunk(&:itself).map { |y, ys| ys.size == 1 ? y : {y => ys.size} }


Parse Bloomberg Excel/CSV with Pandas DataFrame I retrieved  Bloomberg data using the Excel API.  In the typical fashion, the first row contains tickers in every fourth column, and the second row has the labels Date, PX_LAST, [Empty Column], Date, PX_LAST, etc.  The following rows have dates and last price.

   EHFI38 Index BBGID,                   , , EHFI139 Index BBGID,                    , ...
   Date              , PX_LAST           , , Date               , PX_LAST            , ...
   1999-12-31        , 100.0000          , , 1999-12-31         , 100.0000           , ...
   2000-01-31        , 100.1518          , , 2000-01-31         ,  98.6526           , ...
...

It seems that the proper data structure would be a DataFrame with dates as the index, and tickers as the column names.

    , Date,       EHFI38 Index BBGID, EHFI139 Index BBGID, EHFI139 Index BBGID, EHFI84 Index BBGID, ...
   0, 1999-12-31, 100.0000          , 100.0000           , 100.0000           , 100.0000, ...
   1, 2000-01-31, 100.1518          ,  98.6526           ,  98.6526           , 104.7575, ...
...

I wrote this code, which seems to work when I step through it, but I'm sure I'm not doing it well.  I'd like to learn how to do it better.

# IMPORT
import pandas as pd
import numpy as np
import datetime

# READ IN CSV FILES
# EHFI38 Index BBGID,                   , , EHFI139 Index BBGID,                    , ...
# Date              , PX_LAST           , , Date               , PX_LAST            , ...
# 1999-12-31        , 100.0000          , , 1999-12-31         , 100.0000           , ...
# 2000-01-31        , 100.1518          , , 2000-01-31         ,  98.6526           , ...
# ...
px = pd.read_csv('Book1.csv', sep=',', parse_dates=True)

# REMOVE EMPTY COLUMNS
px = px.dropna(axis=1, how='all')

# CONVERT TO ARRAYS
M = np.array(px)
C = np.array(px.columns)

# FIX UNNAMED COLUMNS IN C
for i in arange( len(C)/2 ) * 2:
    C[i+1] = C[i]

# CONVERT EXCEL DATES FUNCTION (THANKS JOHN MACHIN)
def xl2pydate(xldate, datemode):
    # datemode: 0 for 1900-based, 1 for 1904-based
    return (
        datetime.datetime(1899, 12, 30)
        + datetime.timedelta(days=xldate + 1462 * datemode)
    )

# CONVERT DATES THE UGLY WAY
# LOOP THROUGH 1,2, ... last row
for i in arange( len(M)-1 ) + 1:

    # LOOP THROUGH 0,2, ... last column-1
    for j in arange( len(M.T)/2 ) * 2:

        # CONVERT DATE & STORE
        if isinstance(M[i,j],str) and M[i,j].isdigit():
            M[i,j] = xl2pydate(int(M[i,j]), 0)
        else:
            M[i,j] = NaN

# RECOMBINE IN A DATAFRAME
df = pd.DataFrame(M[1:,:], columns=[C,M[0,:]])

# MERGE DATES
#  , Date,       EHFI38 Index BBGID, EHFI139 Index BBGID, EHFI139 Index BBGID, EHFI84 Index BBGID, ...
# 0, 1999-12-31, 100.0000          , 100.0000           , 100.0000           , 100.0000, ...
# 1, 2000-01-31, 100.1518          ,  98.6526           ,  98.6526           , 104.7575, ...
# ...

# LOOP 0,2,...,len-1
for i in arange( (len(df.T))/2 ) * 2:

    # GET A DATE, LAST_PX FOR A SINGLE TICKER
    b = df[df.columns[i:(i+2)]]

    # CHANGE COLUMN NAMES TO DATE, [TICKER]
    b.columns = (df.columns[i][1], df.columns[i][0])

    # COMBINE
    if i==0:
        a = b
    else:
        a = pd.merge(a.dropna(), b.dropna(), on='Date', how='outer')

You can probably do most of what you want in native Pandas.
It has functions for excel file IO that will probably take care of much of the date-munging.

If you want to carry on with the intermediate .csv file, the following should help.

Because you have an empty column and 3 commas between EHFI38 Index BBGID and EHFI139 Index BBGID, your data are in a slightly strange format.

import pandas as pd
from cStringIO import StringIO

data = """\
EHFI38 Index BBGID,                   , , EHFI139 Index BBGID,
Date              , PX_LAST           , , Date               , PX_LAST
1999-12-31        , 100.0000          , , 1999-12-31         , 100.0000
2000-01-31        , 100.1518          , , 2000-01-31         ,  98.6526
"""
df = pd.read_csv(StringIO(data), header=[0, 1], parse_dates=True)
df

There should be a way to sort out the strange indexing but I cannot easily work it out. Try searching for pandas multi-index and hierarchical data.


Running a shell command when a file is added to FTP on Linux Ubuntu I've recently set up a continuous integration pipeline to deploy a .NET Core application to my Linux machine living on the cloud.

My CI pipeline publishes my application, (specifically a .NET web application), zips it and then uploads by ftp.

Now, I've developed the following shell script which I will run as a service.

This script is able to detect as soon as a file has been uploaded to
my FTP folder (/home/MyName/ftp/files) by tailing the vbftp log.
The script then stops the service running the actual web
application, taking the site temporarily offline.
The existing site is then backed up.
We then unzip the newly uploaded file to the location where the web
app lives.(/var/www/ MyProject).
The newly uploaded file is deleted and the service restarted.

Below are the entire contents of the shell script

#!/bin/sh

tail -F /var/log/vsftpd.log | while read line; do
  if echo "$line" | grep -q 'OK UPLOAD:'; then
    filename=$(echo "$line" | cut -d, -f2)
    if [ "$ filename" == "MyProject" ]; then
     # Stop site service
     service MyProject stop
     # Make backup of existing files
     sudo zip -r /var/backups/site/$(date +%F)_MyProject /var/www/MyProject
     # unzip the newly received files
     # (-o to overwrite only the files which have changed)
     sudo unzip -o /home/MyName/ftp/files/MyProject.zip -d /var/www/MyProject
     # Remove uploaded zip
     sudo rm /home/MyName/ftp/files/MyProject.zip
     #Restart service
     service MyProject start
  fi
done

I'm somewhat new to Linux and shell and would love some feedback on the script I've thus far produced above.

[ x == y ] is not strictly-conforming POSIX sh. Use [ x = y ] instead.

Instead of using /home/MyName, you may consider using the environment variable ${HOME}.

I think you made a typo in "$ filename". Use "$filename" instead.

Good job on the commenting and indentation.