Using a navigation menu to help keep material organized I'm working on a personal project to keep different snippets/examples/small projects of mine organized. I want to make the most of my page width, so I decided to write a navigation menu that slides out. It works as expected, but the code is kinda big.

I provided the HTML/CSS just in case that helps, but I'm really only looking for help with the JavaScript. I don't need anyone to rewrite the whole thing.

$(document).ready(function() {
 $('#tab').click(function() {
  if($('#main').attr('class') != 'out') {
     $('#main').animate({
      'margin-left': '+=340px',
       }, 500, function() {
       $('#main').addClass('out');
       });

    } else {
      $('#main').animate({
       'margin-left': '-=340px',
        }, 500, function() {
       $('#main').removeClass('out');
        });

     }
    });

    //does the same as above, but when I press ctrl + 1. from /http://www.scottklarr.com/topic/126/how-to-create-ctrl-key-shortcuts-in-javascript/
    $(document).keyup(function (e) {
     if(e.which == 17) isCtrl=false;
      }).keydown(function (e) {
       if(e.which == 17) isCtrl=true;
       if(e.which == 97 && isCtrl == true) {
        if($('#main').attr('class') != 'out') {
           $('#main').animate({
            'margin-left': '+=340px',
             }, 500, function() {
             $('#main').addClass('out');
             });

          } else {
            $('#main').animate({
             'margin-left': '-=340px',
              }, 500, function() {
             $('#main').removeClass('out');
              });

           }
            return false;
           }
    });
   });
#sidebar {
 top: 0;
 left: 0;
 position: relative;
 float: left;
 }

#main {
 width: 320px;
 padding: 10px;
 float: left;
 margin-left: -340px;
 }

#tab {
 width: 30px;
 height: 120px;
 padding: 10px;
 float: left;
 }

.clear { clear: both; }

First, it seems that isCtrl is not declared anywhere in your code. Put var isCtrl = false; as your first line inside the ready function. Otherwise, you will get a JavaScript error when the first key the user presses is not the Ctrl key. Also, == true is unnecessary within an if statement; it can be omitted.

Second, this line could be improved, as it fails if the element is a member of another class:

if($('#main').attr('class') != 'out') {

It can be rewritten using .hasClass() as:

if(!$('#main').hasClass('out')) {

In fact, you should refactor this entire block out to a separate function to avoid duplicating code:

function toggleMenu() {
    if(!$('#main').hasClass('out')) {
        // ...
    } else {
        // ...
    }
}

And put toggleMenu(); where the duplicate code was in each function. Also, if performance is a concern, you should cache the jQuery object $('#main') by declaring another variable at the beginning of the ready function. Not doing so is slow because jQuery would have to find the matched elements within the document again.

A more minor criticism is that of braceless control statements, such as:

if(e.which == 17) isCtrl=false;

Some say that it is best to always put the code within indented braces to avoid errors caused by incorrectly placed semicolons.

Finally, the indentation doesn't look right throughout the entire code. Fix that, and your code will become somewhat more readable.

To add: It also looks like you have an extra comma at the end of:

'margin-left': '+=210px',

This is syntactically valid, but it causes problems with Internet Explorer 7 and below.


Return a string based on Type, Color and Position I recently wrote a really long one-liner, it was really just for fun, but I would like to know if it's bad practice or not. Can you tell me?

Here's my full class, but it's about the ToString() method.

namespace HGroupBadge.Classes
{
    public class BadgeDetail : BadgePart
    {
        public DetailType Type { get; set; }

        public BadgeDetail(DetailType type, BadgeColor color, BadgePosition position)
        {
            Type = type;
            Color = color;
            Position = position;
        }

        public override string ToString()
            => ((int)Type > 99 ? "t" : "s")
            + ((int)Type > 99 ? ((int)Type).ToString().Remove(0, 1) : ((int)Type < 10 ? "0" + (int)Type : "" + (int)Type))
            + ((int)Color < 10 ? "0" + (int)Color : "" + (int)Color)
            + (int)Position;
    }
}

It has to return a string similar to s01137, when Type or Color are lower than 10, a leading 0 has to be added because the final string always has to end in 5 digits.  When Type is bigger than 99, the first digit has to be cut off. In the end, Position has to simply be added to the string.

DetailType, BadgeColor and BadgePosition are enums.

public override string ToString()
    => ((int)Type > 99 ? "t" : "s")
    + ((int)Type > 99 ? ((int)Type).ToString().Remove(0, 1) : ((int)Type < 10 ? "0" + (int)Type : "" + (int)Type))
    + ((int)Color < 10 ? "0" + (int)Color : "" + (int)Color)
    + (int)Position;

You definitely should split it in a exactly four methods/variables:

Method 1: GetPrefix

((int)Type > 99 ? "t" : "s")

Method 2: FormatSomething, sorry, I don't know what.

((int)Type > 99 ? ((int)Type).ToString().Remove(0, 1) : ((int)Type < 10 ? "0" +  (int)Type : "" + (int)Type))

Method 3: FormatColor:

((int)Color < 10 ? "0" + (int)Color : "" + (int)Color)

Method 4: FormatPosition - this might be unnecessary but because all other formattings are method let's make this consistant.

(int)Position

Finally your ToString would look like this:

public override ToString() =>
    $"{GetPrefix()}{FormatSomething()}{FormatColor()}{FormatPosition()}"

and nobody asks any more questions about what your method is doing.


Polynomial class C++11 Initial stage of implementation of the polynomial class. Essentially I need to implement the basic mathematical operations. Right now I have implemented just addition. Was wondering if this is a good way to proceed. I also want to be sure if the move constructor would be called during the addition operation.

poly.h

/* Class for polynomials*/
/* Constant term is the first coefficient*/
/*To Do:
1. Establish invariants

*/
#ifndef POLY_H
#define POLY_H
#include <vector>
#include <iostream>
#include <cstdlib>
#include <stdexcept>
template< typename T>
class Poly
{
 public:
 Poly() = default; // default constructor
 Poly(size_t s):sz{s}, coeff{}{}; // constructor assumes positive size
 Poly(std::initializer_list<T> lst):sz{lst.size()}, coeff{lst}{}; // constructor with initializer list
 Poly(Poly && ) = default; // Move
 Poly &  operator = (Poly && ) = default;
 Poly (Poly const & ) = default; // Copy
 Poly &  operator = (Poly const & ) = default;

 T & operator[](int i) {
      if(i < 0 || i >= sz)
     throw std::out_of_range{"Poly::operator[]"};
   return coeff[i];
 }

 size_t getSize() const{return sz;};

 Poly &  operator += (const Poly &b){
   size_t bsz = b.getSize();
   if(sz > bsz){
     for(int i = 0; i < bsz; ++i)
       coeff[i] += b[i];
   }
   else{
     int i = 0;
     for(; i < sz; ++i)
       coeff[i] += b[i];
     sz = bsz;
     for(; i < bsz; ++i)
       coeff.push_back(b[i]);
   }
   return *this;
 }
 const T & operator[](int i) const{
      if(i < 0 || i >= sz)
     throw std::out_of_range{"Poly::operator[]"};
   return coeff[i];
 }

 T * begin(){return & coeff[0];}
  T * end(){return & coeff[0] + sz;}

 ~Poly() = default;

 private:
 std::size_t sz;
 std::vector  <T>  coeff; // coeff is a vector of type T. It could be the class Poly itself
 };
template <typename T>
inline Poly < T >operator + (const Poly < T > & a,  const Poly < T > & b){
  Poly < T > res{a};
  res += b;
  return res;
}

#endif /* POLY_H*/

main.cpp (for test runs)

#include <iostream>
#include <vector>
#include "poly.h"
int main(int argc,  char **argv)
{
  std::vector  < double > a;
  a =  {1, 2, 3};

  Poly < double > poly1{2, 3, 4, 5};
  Poly < double > poly2{2, 3, 4};
  poly1 += poly2;
  for(auto & x: poly1)
    std::cout << x;

  poly1 = poly2 + poly2;
  for(auto & x: poly1)
    std::cout << x;

  return 0;
}

Overall, this looks good, but I think there are a few places it's still open to a little bit of improvement.

Consistency

You're somewhat inconsistent about the type you use for the size (and index into) a polynomial--you have a constructor that takes a size_t, but subscript operator that takes an int. I'd choose one type, and use it consistently. Lacking a really specific reason to do otherwise, I'd use size_t throughout (or see below, and use size_type instead--it's probably a better choice than either size_t or int).

This can also simplify your code a bit. For example, you subscript operator checks for an argument less than zero, but that simply can't exist with size_t.

Consider conforming to container interface

Right now, your Poly type can act quite a bit like a container (e.g., it has begin(), end(), and operator[] just like a container that supports random access iteration. It's fairly trivial to add a few more typedefs to make it a reasonably complete implementation:

typedef T value_type;
typedef T &reference;
typedef std::vector<T>::iterator iterator;
typedef std::vector<T>::const_iterator const_iterator;
typedef std::vector<T>::difference_type difference_type;
typedef std::vector<T>::size_type size_type;

The standard's requirements for containers are spelled out in §[container.requirements] (aka §23.2 in recent drafts), especially Tables 96, 97 and 98.

Use iterators where applicable

This is almost part of the previous point, but (I think) bears being pointed out separately.

Rather than defining begin() and end() to just return pointers, I'd define them to return iterators. I'd at least consider adding the rest of the normal rbegin(), cbegin(), crbegin() (and matching *end()) functions as well.

Use the right member functions

That leads directly to the point: once you've changed its argument to a size_t, your operator[] can be:

T & operator[](size_t i) {
    if (i >= sz)
        throw std::out_of_range{ "Poly::operator[]" };
    return coeff[i];
}

This, in turn, reduces to:

T & operator[](size_t i) {
    return coeff.at(i);
}

I'd also at least consider following the same path as vector, and providing a .at() that does bounds checking and an operator[] that doesn't.

Code arrangement

I'd rather see the two overloads of operator[] right next to each other in the code (without getSize() and operator+= between them). [Yes, this is a pretty minor point.]

Don't repeat yourself

Right now, you operator+= has two legs that start with nearly identical loops:

    if (...) {
        for (int i = 0; i < bsz; ++i)
            coeff[i] += b[i];
    }
    else {
        int i = 0;
        for (; i < sz; ++i)
            coeff[i] += b[i];

        // ...
    }

I'd rather eliminate that duplication. Personally, I'd probably do that by changing the order of operations a little bit. First, I'd resize my vector to the larger of the two sizes, then I'd do the addition:

coeff.resize(std::max(sz, bsz));
for (size_type i=0; i<sz; i++)
    coeffs[i] += b[i];

Alternatively, you could do something like this:

for (size_type i=0; i<sz; i++)
    coeffs[i] += b[i];
for (size_type i=sz; i<bsz; i++)
    coeffs.push_back(b[i]);

In this case, if sz >= bsz, the second loop won't execute (or: "will execute zero iterations"). To work with this, however, you also have to deal with the fact that it could index into a nonexistent element in the b. One way to deal with that would be to rewrite operator[] a bit to treat out of bounds accesses as legitimate, but with a zero coefficient:

T operator[](size_type i) {
    if (i > sz)
       return T{};
    return coeffs[i];
}

So this is basically observing that a polynomial like \$A + bX^2\$ can be treated as \$A + bX^2 + 0x^3 + 0x^4 + ...\$. Obviously this is mathematically fine--since we're giving 0 as the coefficient for any higher order terms, they don't affect the overall value. This way, the right-hand operand of our += can be treated as always having at least as many terms as the left hand operand.

If wanted to get even fancier, we could do roughly the same with T &operator[] as well, but that gets quite a bit trickier, since it would (potentially) have to reallocate the vector of coefficients. At least in my opinion, that's probably more trouble than it's worth (but, I'll repeat: I'd prefer to resize first, and be done with it--it keeps everything simple and neat).

Move Constructor

As far as your question about use of the move constructor goes, the answer to precisely the question you've asked ("I also want to be sure if the move constructor would be called during the addition operation.") is "no".

The move constructor won't (at least normally be called during an addition operation. Rather, it'll be used after the addition operation, to move value from the temporary that's created in the addition, into the destination being initialized from that addition. If you're assigning to that destination rather than initializing it, the move assignment operator will be used instead.

As far as the question you really intended to ask though: yes, these will be used to move the result of an addition into a destination. This is fairly easy to verify by changing your implementation a bit to add some instrumentation:

Poly(Poly &&src) : sz(src.sz), coeff(std::move(src.coeff)) {
    std::cout << "\nMove ctor\n";
}
Poly &  operator = (Poly &&src) {
    sz = src.sz;
    coeff = std::move(src.coeff);
    std::cout << "\nMove assign\n";
    return *this;
}

The result when running your main.cpp is:

4685
Move ctor

Move assign
468

Showing that yes, the move ctor and move assignment operator were actually used.


Weather application data structures and design Relevant files are script.ts, WeatherData.ts, index.html.

I made a simple weather 5-day forecast application using mainly TypeScript. I was wondering how I can improve my code since I originally wanted to use a data structure that I created called a "WeatherData" to store my information about the weather for each day of the week.

GitHub project

WeatherData data structure

export class WeatherData {
    date: string;
    city: string;
    country: string;
    temperature: number;
    minTemperature: number;
    maxTemperature: number;
    weather: any;
    weatherIcon: any;
}

TypeScript file concerns

The main concerns I have about the TypeScript files (script.ts, WeatherData.ts) is the fact that halfway through making my project, I realized that I didn't actually need to use a WeatherData data structure. This led to me passing in WeatherData variables in functions as well as returning WeatherData variables from functions that don't actually need to return it.

Is there a more object-oriented way I should be approaching my script.ts class in order to use data structures?
Will using a WeatherData data structure be better?
How can I format my code (maybe different classes?) such that all the functions aren't all clumped together?

HTML file concerns

First time ever making a front-end UI using HTML and I don't know what areas I should refactor. I noticed I have a lot of repeated element tags such as:

Day 0

month-day 0

Forecast

Current

Max

Min

I have 5 of these (one for each day).

Screenshot of what the UI looks like:

What type of suggestions are there to making my UI look better? I was thinking of making the jumbotron element a different color.

About the HTML

You shouldn’t use heading elements (h1-h6) like that. These elements give the heading for the section they create. Don’t use them for the name in simple name-value items, nor for stand-alone content (where no other content in the section exists).

You could represent each day as a section:

A section should ideally have a heading (and it’s the only one needed in this case). I think it makes the most sense to use the day+date as heading. The time element can be used to mark up the date in a machine-readable way. The abbr element can be used to give the full name of the day.

Sun <time datetime="2017-12-31">Dec. 31</time>

The forecast can be given in a p. As the img seems to convey the same content, it’s correct to keep the alt attribute empty.

clear sky

While you could use a list for the temperatures, I think three p elements are sufficient in this simple case.

14 °C

High: 20 °C

Low: 6 °C

When using a label for the first temperature, too, I would go with a dl element instead:

Current

14 °C

High

20 °C

Low

6 °C

So, an entry for a day could look like this:

Sun <time datetime="2017-12-31">Dec. 31</time>

clear sky

14 °C

High: 20 °C

Low: 6 °C

or with dl:

Sun <time datetime="2017-12-31">Dec. 31</time>

clear sky

Current

14 °C

High

20 °C

Low

6 °C


JS to create Table data dynamically The following codes generate table row dynamically. I wonder if there are better ways of generating the same data. Also instead of manually coding the heading and the table itself, is it possible to code them with less codes without using the innerHTML function.

            ID
            Name
            Mark Score

Data:

const students = {
        '24': {
            'id':'24',
            'Name':'Zuali',
            'mark':30,
        },
        '25': {
            'id':'25',
            'Name':'Famkima',
            'mark':52,
        },
        '27':{
            'id':'27',
            'Name':'Duha',
            'mark':77,
        },
        '28':{
            'id':'28',
            'Name':'Rema',
            'mark':81,
        },
        '29':{
            'id':'29',
            'Name':'Sanga',
            'mark':47,
        },
        '30':{
            'id':'30',
            'Name':'Dari',
            'mark':23,
        },
    };

JS code to create the table data:

const markObtained = Object.keys(students).reduce(function(names, name) {
if (students[name].mark>=20) {
    names[name] = students[name];
}
return names;
}, {});
const doc = document.createDocumentFragment();
const table = document.querySelector("table");
for(let i=0; i<Object.keys(markObtained).length; i++) {
    const data = Object.values(markObtained)[i];
    const tr = document.createElement("tr");
    for (const v in data){
         const td = document.createElement("td");
        td.textContent = data[v];
        tr.appendChild(td);
    }
  table.appendChild(tr);
 }
table.appendChild(doc);

I would highly appreciate if any expert in js can help me better optimize the codes and give me another workaround examples?

Minor problems and style points

The table has named columns set out already. This is problematic as the for in loop is alphabetic and thus you would be adding Name, id, mark which does not match the column headings.

It would be best to have the student fields defined as an array in the order that they are to be displayed. This can either be as dataset properties of the array or in the source code, or added to the dataset. The example has it in the source (be cause I am lazy)
Use for of rather than for(;;) and try to avoid ever using for in as it has a long list of caveats and problems associated with its use.
The function markObtained...

...seems redundant in the example you give. The data can be accessed just as easily in one pass.
...the naming is confusing withing the reduce call. names, and name are both referring to the student id. This is especially problematic as the dataset has a field called Name (which should not be capitalized)
...creates an object indexed by id, the dataset you have shown is also an Object. Both are indexed via student id, yet you don't ever use the student id to lookup a student. markObtained would be best to return an array of student id's rather than copy the student references to a new object. See Example A

The document fragment your create is not used, the last line is adding an empty fragment to the table.
A table has insertRow(), and rows have insertCell(), use them to add to the table rather than create them as DOM element via document.createElement

Extract ids

From above points regarding the function markObtained The example creates an array of student ids that can be used to locate students in the original dataset. Also renames the variables so not to be confusing.

Example A

const markObtained = Object.keys(students).reduce(function(ids, id) {
    if (students[id].mark>=20) { ids[id] = id }
    return ids;
}, []);

Adding to DOM

You are appending to the table that is on the page. Each time you add a cell or row you are forcing a re flow.

You can either create the whole table in code and add it to the DOM when ready, or you can remove the table from the DOM, modify it and put it back see Example B.

If you remove and then put back it would be best to put the table within a container to ensure it goes where it belongs without undue fuss.

Example B

const students = {
    "24": {id: 24, name: "Zuali", mark: 30},
    "25": {id: 25, name: "Famkima", mark: 52},
    "27": {id: 27, name: "Duha", mark: 77},
    "28": {id: 28, name: "Rema", mark: 81},
    "29": {id: 29, name: "Sanga", mark: 47},
    "30": {id: 30, name: "Dari", mark: 23},
};

const PASS_MARK = 20;
const FIELDS = ["id", "name", "mark"];
updateStudentsTable(students);

function addPassed(table, students, pass = PASS_MARK) {
    for (const student of students) {
        if (student.mark >= pass) {
            const row = table.insertRow();
            for (const field of FIELDS) {
                row.insertCell().textContent = student[field];
            }
        }
    }
}
function updateStudentsTable(students) {
    const table = document.querySelector("table");
    const par = table.parentElement;
    par.removeChild(table);
    addPassed(table, Object.values(students));
    par.appendChild(table);
}

                Id	Name	Mark


Compress svg files in PHP I wrote something to "compress" svg files. The svg files I am using often have comments and empty <g> tags, and I want to remove them. My main goal is not the speed of the compression, but the size of the compressed svg file.

Here is an example svg file: https://image.flaticon.com/icons/svg/222/222436.svg

And here is the code I am using:

public function compress($svg)
{
    $svg = preg_replace('/<!--.*-->/', '', $svg);
    $svg = preg_replace('/<g>[\n\r\s]*<\/g>/', '', $svg);
    $svg = preg_replace('/\n/', ' ', $svg);
    $svg = preg_replace('/\t/', ' ', $svg);
    $svg = preg_replace('/\s\s+/', ' ', $svg);
    $svg = str_replace('> <', '><', $svg);
    $svg = str_replace(';"', '"', $svg);

    return $svg;
}

Do you see any dangers here, perhaps this could ruin some svg files?
Is there something I could do to compress the file even more?
Is there any way to speed this up?

SVG is structured data and should therefore not require regex to work with it.  I would suggest that you work with either an XML or SVG-specific library in PHP which would let you parse in the input SVG, remove any unwanted nodes and then render it back out.

Here is a starting point for common PHP tools for working with XML that should be able to do what you want.

This also looks like a library that could meet your needs.


SEDE query to find users with exactly one post in a particular tag I'm doing some research for a meta post on Gardening.SE (where I'm a pro tempore moderator). I'm using the Stack Exchange Data Explorer (SEDE) to find information about first time users who post questions in our [identification] tag, and how they subsequently interact with the people who respond to their question, whether that's comments on the post, comments on answers, editing the post to provide more information, and so on.

I have the following query which gives me the list of people with no answers and one question that includes the [identification] tag. I plan to refine it by adding extra criteria, e.g. to find instances where the OP leaves a comment replying to someone else.

-- Find users who have exactly one post
-- with that post being a question
-- and the question has the [identification] tag
SELECT
  Users.Id AS [User Link], Posts.Id AS [Post Link]
FROM
  Users INNER JOIN Posts ON Posts.OwnerUserId = Users.Id
        INNER JOIN PostTags ON Posts.Id = PostTags.PostId
        INNER JOIN Tags ON Tags.Id = PostTags.TagId
WHERE
  Tags.TagName LIKE 'identification' AND
  Posts.ParentId IS NULL AND   -- ParentId is NULL if post is a question
  Users.Id in (
    -- Users with exactly one question
    SELECT Users.Id
    FROM
      Users INNER JOIN Posts ON Posts.OwnerUserId = Users.Id
    WHERE
      Posts.ParentId IS NULL
    GROUP BY
      Users.id
    HAVING
      Count(Posts.Id) = 1
    ) AND
  Users.Id NOT IN (
      -- Users with 1 or more answers
    SELECT Users.Id
    FROM
      Users INNER JOIN Posts ON Posts.OwnerUserId = Users.Id
    WHERE
      Posts.ParentId IS NOT NULL
    GROUP BY
      Users.id
    HAVING
      Count(Posts.Id) > 0
    )
ORDER BY
  Users.Id DESC  -- newest users first.

My concern is that this SQL is far from optimal. I'm looking especially at the repeated inner joins between the Users and Posts tables, and the similarity between the sub-queries to find users with one question and users with at least one answer.

Gardening.SE is a small site, so this query consistently takes less than 75ms to return its 101 records, but even so I'd like to make it run more efficiently, if possible. As I said, I plan to enhance this query, which will mean pulling in more tables, so I'm also concerned about it scaling that way.

Data Explorer links:

Users with one [identification] question and no answers — the above query.
Drive-by users with one [identification] question — a derived query for users who post and are never seen again.

Half of the tricks with optimizing SQL is knowing the database schema.

The schema documentation can be found in this Meta post: Database schema documentation for the public data dump and SEDE.

There are four functional issues I see with your query, and a couple of suggestions after that:

Use PostTypeId - it describes what the Post is (Question == 1, Answer == 2) and is well indexed.
you are querying closed posts (which have a non-null ClosedDate)
you join to Tags for a single record. This can be resolved independantly
you have Posts three times in your query, and it can be there only twice if you do double-duty in the 'Only One Question' part of the query....

As for the suggestions:

I like parametrize things, and being able to change the tag you are interested makes sense, so I would parametrize that.
The SQL Server syntax allows for 'with' sections in a query, and that makes the code a lot more readable than the query-in-from-clause syntax

Using these suggestions I restructured your query to do a few things:

add a User-Name sort column (SEDE does not sort [User Link] columns well)
parametrized the tag, with a default value of identification
made the group-by and having a much smaller component in the query.
added the closed-query support (and community-wiki - if there are).

I forked your query here... ... for me it ran through in 33 milliseconds

-- Find users who have exactly one post
-- with that post being a question
-- and the question has the [xxxx] tag (default identification)

declare @tag as nvarchar(25) = ##tag:string?identification##
;
declare @tagid as int
;

-- get the tag id here so it is not needed as a join table
select @tagid = Id
from Tags
where TagName = lower(@tag)

;

print 'Tag for ' + @tag + ' is ' + Convert(NVarchar(10), @tagid)

;

with Answerers as (

    select distinct OwnerUserId as [UserId]
    from Posts p
    where PostTypeId = 2 -- answers
      and CommunityOwnedDate is null -- not CW
      and ClosedDate is null -- not closed

), SingleQ as (

    Select OwnerUserId as [UserId],
           Min(Id) as [FirstPost],
           Sum(Score) as [Score],
           count (*) as [QCount]
    from Posts p
    where PostTypeId = 1 -- question
      and CommunityOwnedDate is null -- not CW
      and ClosedDate is null -- not closed
      and not exists (
            select UserId
            from Answerers
            where UserId = OwnerUserId) -- active answerers.
    group by OwnerUserId
    having count(*) = 1
)

SELECT
  ROW_NUMBER() OVER (order by Users.Id Desc) as [Recent],
  Users.Id AS [User Link],
  Users.DisplayName AS [Sort By Name],
  SingleQ.FirstPost AS [Post Link],
  SingleQ.Score AS [Score]
FROM Users
     INNER JOIN SingleQ ON Users.Id = SingleQ.UserId
     INNER JOIN PostTags ON SingleQ.FirstPost = PostTags.PostId
WHERE PostTags.TagId = @tagid
ORDER BY
  Users.Id DESC  -- newest users first.


Hide menu during user inactivity with jQuery, setInterval performance I created a script to hide a menu during user inactivity and when the position of my page is left < 2.

I would like to know if I use a good method regarding the performances. It's not too heavy ? Should I optimize my code ?

var interval = 1;

setInterval(function(){
    var posx=$("#page").offset();
    if (interval == 30 && posx.left < 2) {
    $('#menu').fadeOut(1000);
       interval = 1;
   }
   interval = interval+1;
    console.log(interval);
    },1000);

$(document).bind('mousemove keydown scroll touchstart', function() {
    var posx=$("#page").offset();
    if($('#menu').is(':hidden') && posx.left < 2) {
        $('#menu').fadeIn(1000);
        interval = 1;
    }
});

Hmmm... polling at regular intervals instead of using a handler... not good. It means that the interval between the event (mousemove, etc) and the call to fadeOut is variable. It also means that the script continues to use up a small amount of CPU while nothing is happening. And there's a tradeoff between these two things: If you increase the second argument to setInterval you get more variation in waiting time but less needless CPU usage.

Also, the logic feels slightly wrong: the element is hidden if it is near the left edge after 30 seconds; but if it isn't, and subsequently is moved left, fadeOut will not be called. Having said that, I'm not sure what the events are; if it's impossible to move it left without triggering one of the events, then ignore this paragraph. If this is an issue then I'm not sure exactly what behaviour you want, so I've ignored this issue in my answer - my code will behave as I just described.

Anyway, what you need is window.setTimeout; this function schedules another function to run after a given interval. So if you call window.setTimeout in the event handler, it will cause another function (hideIfNecessary, below) to run 30 seconds after an event is fired.

Of course, if another event is fired while the clock is ticking, you want to call hideIfNecessary 30 seconds after the second event, but cancel the one 30 seconds after the first event. To cancel events scheduled with window.setTimeout, use setTimeout's evil twin, clearTimeout.

So without further ado, here's my version of your code with setTimeout. (I haven't tested it though.)

var timeoutId = null;

function hideIfNecessary(){
    var posx=$("#page").offset();
    if (posx.left < 2) {
        $('#menu').fadeOut(1000);
    }
}

$(document).bind('mousemove keydown scroll touchstart', function() {
    var posx=$("#page").offset();
    if($('#menu').is(':hidden') && posx.left < 2) {
        $('#menu').fadeIn(1000);
    }
    if (timeoutId !== null) window.clearTimeout(timeoutId);
    timeoutId = window.setTimeout(hideIfNecessary, 30000);
});


Calculating user efficiency on a time period with Linq I have this expression:

    public static readonly Expression<Func<ApplicationUser, decimal>> EfficiencyPercentExpr = e =>
        (e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30)).Count(x => x.Status != Order.OrderStatus.Deleted) > 0)
        ?
        (
        e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30)).Count(x => x.Status != Order.OrderStatus.Deleted)
        /
        e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30)).Count()
        )
        :
        0
    ;

This expression calculates the user's efficiency by calculating the percentage of successful orders from a total of the last 30 days.

How can I make it more clear?

Update:

There is a second approach of writing the expression, but it generates more expensive SQL query.

    private static readonly Expression<Func<ApplicationUser, decimal>> efficiencyPercentExpr = e =>
        e.ExecutorOrders.Where(x => !x.IsNotConsidered && x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30))
        .GroupBy(x => 1, (k, g) => new { Count = g.Count(), CountUndeleted = g.Count(x => x.Status != Order.OrderStatus.Deleted) })
        .Select(x => x.CountUndeleted > 0 ? (decimal)x.CountUndeleted / (decimal)x.Count : 0).FirstOrDefault();

Don't one line things that shouldn't be one lined. Just because you can do something, doesn't mean that you should.
Extract the duplicated query into a method of it's own that returns an IQueryable
Don't query the database twice for the same count, execute the query and store it in a variable.

private IQueryable<foo> ExecutorOrdersInLast30Days(ApplicationUser e)
{
    return e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30));
}

private decimal EfficiencyPercent(ApplicationUser e)
{
    var nonDeletedOrders = ExecutorOrdersInLast30Days(e).Count(x => x.Status != Order.OrderStatus.Deleted);
    if ( nonDeletedOrders > 0)
    {
        return nonDeletedOrders / ExecutorOrdersInLast30Days(e).Count();
    }
    else
    {
        return 0;
    }
}


GUI layout for a security camera app I'm designing a security camera program.  Right now, I am just trying to set up the GUI before receiving. I am using Kivy since, in the future, I plan on designing some programs for Android.  I really like Python, and I figured I'd start learning Kivy.

My question is: am I building the GUI structure correctly?

The layout I have planned is for the core container to have a BoxLayout that will hold three containers:

The first container is on the left and will hold buttons to perform various commands.
The second container will be in the center and will hold all the video feeds.
The third container will be on the right and its purpose is still being considered.

import kivy
from kivy.config import Config
Config.set('graphics', 'fullscreen', 'auto')
from kivy.app import App
from kivy.uix.label import Label
from kivy.uix.button import Button
from kivy.uix.boxlayout import BoxLayout
from kivy.uix.anchorlayout import AnchorLayout

class camMonitor(BoxLayout):

    def __init__(self, **kwargs):
        super(camMonitor, self).__init__(**kwargs)
        self.add_widget(controlInterface())

class controlInterface(BoxLayout):

    def __init__(self, **kwargs):
        super(controlInterface, self).__init__(**kwargs)
        button1 = Button(text='action 1', size =(50,30), size_hint=(None,None))
        self.add_widget(button1)

class camMonApp(App):

    def build(self):
        return camMonitor()

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

For each container, I have created a separate class. Not every class will be a box layout; that's just what I have it set to at the moment.

Should I continue building the GUI the way it is now, or should everything be put into one class?

tl;dr: If you are planning on adding more methods to each of these classes, then use them. If not, then don't.

One of the first things you should check when deciding whether to use a class or not, is if the class has two or less methods, __init__, and another function. Since your code doesn't like complete quite yet, I'm making the assumption that there will be more functions added to each of these classes.

Now, I do have a few things about your code that I want to point out. Here's a list of small things you could improve.

Class names should be in PascalCase. Not lowerCasePascalCase.
There should be two blank lines between each class/function/code block on the top level of your file.
You have a blank line before each of your class declarations. Preferably, you should either, put a docstring here, or, remove the blank line.

If you want to make sure that your code is PEP8 compliant, you can visit PEP8's page here, or use this online PEP8 checker here. If you want a checker on your local machine, you can just run pip install pep8, and to update, you can run pip install --upgrade pep8.


Project Euler #4 - finding the largest palindrome from the product of two n-digit numbers Project Euler Problem #4: "A palindromic number reads the same both ways. The largest palindrome made from the product of two 2-digit numbers is 9009 = 91 × 99.

Find the largest palindrome made from the product of two 3-digit numbers."

The code works well enough until trying to find the largest palindrome of two eight digit numbers. I found a similar question in C++, but there are some differences:

The main function takes two integers to determine the number of digits of the factors used to produce the products. This allows us to find the largest palindrome for a two digit number and a five digit number or two eight digit numbers.
Two for loops then create the multiplicand and multiplier. I think this could be done better.
The outer for loop iterates over the multiplicand to decrement it each time the inner for loop has completed.
The inner for loop iterates over the multiplier so that inside this loop every possible product from the multiplicand and multiplier are evaluated. This is where I think the largest gains from optimization will come from. I don't think that every product needs to be produced or evaluated to find the largest palindrome nor do I think it requires an inner/outer loop to do so.
The product is then turned into a string so that the first and last values can be compared for equality.
If they are equal the comparison repeats again after removing the compared values until three or less values are left where the first and last values are equal.
The product is then stored as the largest result if it is greater than the previous result.
Finally, the result is returned after all the loops have finished.

I am interested in finding ways to optimize this code.

console.log(largestPalindromeOfTwoFactors(8, 8));
function largestPalindromeOfTwoFactors(multiplicand, multiplier{//Accepts the number of digits of the multipland and the number of digits of         the multiplier
        var  multiplicandString = "";
        var multiplierString = "";
    for(var i = 0; i < multiplicand; i++){
         multiplicandString = multiplicandString.concat(""+9); // 9, 99, 999
    }
    for(var j = 0; j < multiplier; j++){
         multiplierString = multiplierString.concat(""+9); // 9, 99, 999
    }
    var product = 0;
    var result = 0;
    multiplicand = parseInt(multiplicandString); //999, 998...
    for(multiplicand;  multiplicand > 0; multiplicand--){ //Loop 999 times
       multiplier = parseInt(multiplierString); //999, 998...
       for(multiplier; multiplier > 0; multiplier--){//Loop 998001 times
          product = multiplicand * multiplier; //999*999,999*998...998*999,998*998...
          var stringProduct = (''+product);// 90909
          var strLength = stringProduct.length; //Make sure not to modify actual string
          for(strLength; strLength >= 1; strLength--){ //For the length of string
            if( stringProduct[0] == stringProduct[stringProduct.length-1]) //If 90909: 9==9, 090:0==0, 9:9==9
            {
              if(stringProduct.length <= 3){//allow for 090
                if(product > result){ //if product is larger than existing result replace it with new larger number
                  result = product;
                }
              }
              stringProduct = stringProduct.slice(1, -1);//take off the first and last numbers
            }
          }
        }
    }
  return result;
}

Javascript numbers are stored in IEE754 format that can precisely represent numbers up to 2^53 - 1 (9007199254740991), so it's impossible to use the built-in Javascript arithmetic to get actual digits for a lot of two 8-digit number products (for example, 94,906,266 * 94,906,266 exceeds the abovementioned maximum).

There are several BigNumber libraries for JavaScript, but I'll only give an answer for the case of 7 digits and less. A brute-force enumeration with a couple of optimization is quite fast for n<7 compared to the original OP code that took 1370 ms for 3 digits and 3+ minutes for 4 digits.

function largestPalindromeOfTwoFactors(len1, len2) {
    // calculate number range for n digits:  10^(n-1)..10^n-1 (e.g. 10-99)

    // 'a' (outer loop) will be the largest multiplicand
    var maxA = Math.pow(10, Math.max(len1, len2)) - 1 |0;
    var minA = (maxA + 1) / 10 |0;

    // 'b' (inner loop) will be the smallest multiplicand
    var maxB = Math.pow(10, Math.min(len1, len2)) - 1 |0;
    var minB = (maxB + 1) / 10 |0;

    var maxMul = 0, maxMulA, maxMulB;

    for (var a = maxA; a >= minA; a--) {
        // in case len1==len2 only process 'b' smaller than current 'a'
        // as the larger numbers were already checked when used for 'a'
        for (var b = Math.min(a, maxB); b >= minB; b--) {
            var mul = a * b;
            if (mul <= maxMul) {
                // no need to continue, we'll never exceed maxMul with a smaller 'b'
                break;
            }
            var mulStr = mul.toString();
            for (var i = mulStr.length-1, j=0; i>=j && mulStr[i] == mulStr[j]; i--, j++) {}
            if (i <= j) {
                maxMul = mul, maxMulA = a, maxMulB = b;
                // 'a' smaller than current 'b' won't produce a bigger product
                minA = Math.max(minA, b);
            }
        }
    }
    // retain the order of input parameters' magnitudes: 2,3 -> 99, 951
    return [maxMul, len1 < len2 ? maxMulB : maxMulA, len1 < len2 ? maxMulA : maxMulB];
}

Test run:

for (var i=1; i<=7; i++) {
    var start = Date.now();
    console.log('%o in %d ms', largestPalindromeOfTwoFactors(i, i), Date.now() - start);
}

  [9, 9, 1] in 0 ms
  [9009, 99, 91] in 0 ms
  [906609, 993, 913] in 2 ms
  [99000099, 9999, 9901] in 1 ms
  [9966006699, 99979, 99681] in 551 ms
  [999000000999, 999999, 999001] in 171 ms
  [99956644665999, 9998017, 9997647] in 162363 ms <<<<<< ughhhh

Notes:

There are much better algorithms.
090 isn't a number, 90 is, so the former and the likes are not palindromes.


Tally the occurrences of some numbers This actually belongs to a larger interview question. But here it is just the "tally up" part:

var i, a, tally = {}, arr = [1, 3, 7, 9, 3, 1, 6, 3, 3, 2, 11, 2];

for (i = 0; i < arr.length; i++) {
    a = arr[i];
    tally[a] = (tally[a] || 0) + 1;
}
console.log(tally);

I had some feeling the interviewers were not impressed or pleased (or maybe they didn't understand it), when I said I would use the line tally[a] = (tally[a] || 0) + 1; instead of using if... else or the ternary operator to do it.

This is perfectly fine JavaScript, performant even. However...

var i, a, tally = {}, arr = [1, 3, 7, 9, 3, 1, 6, 3, 3, 2, 11, 2];

For better visibility, it's recommended you split up each variable to its own var. Also, to avoid gotchas and IDEs screaming "potentially uninitialized variable" (Intellij does this), initialize them with values.

for (i = 0; i < arr.length; i++) {
  a = arr[i];
  tally[a] = (tally[a] || 0) + 1;
}

Perfectly fine, except it can be better if you use let on i. That way, it strongly indicates that i lives only within the loop body and not anywhere else.

An alternate way to do this is to use array.reduce and carry around the tally object until done. This way, you avoid having to define temporary storage variables and counters.

const arr = [1, 3, 7, 9, 3, 1, 6, 3, 3, 2, 11, 2];
const tally = arr.reduce(function(tally, value){
  tally[value] = tally[value] ? tally[value] + 1 : 1;
  return tally;
}, {});

or more concisely, but entirely unreadable depending on who you ask

const arr = [1, 3, 7, 9, 3, 1, 6, 3, 3, 2, 11, 2];
const tally = arr.reduce((t, v) => (t[v] = (t[v] || 0) + 1, t), {});


Return sum of three values, excluding values within a specified range Is there a way to make my code simpler?

  Given three int values, (a, b, c) return their sum. However, if any of the
  values is a teen -- in the range 13..19 inclusive -- then that value
  counts as 0, except 15 and 16 do not count as a teens. Write a
  separate helper (def fix_teen(n)) that takes in an int value and
  returns that value fixed for the teen rule. In this way, you avoid
  repeating the teen code three times (i.e. "decomposition"). Define the
  helper below and at the same indent level as the main no_teen_sum().

  no_teen_sum(1, 2, 3) → \$6\$
  no_teen_sum(2, 13, 1) → \$3\$
  no_teen_sum(2, 1, 14) → \$3\$

def no_teen_sum(a, b, c):
    if fix_teen(a) == 0 and fix_teen(b) == 0 and fix_teen(c) == 0:
        return 0
    elif fix_teen(a) != 0 and fix_teen(b) != 0 and fix_teen(c) != 0:
        return a + b + c
    elif (fix_teen(a) and fix_teen(b)) != 0:
        return a + b
    elif (fix_teen(b) and fix_teen(c)) != 0:
        return b + c
    elif (fix_teen(a) and fix_teen(c)) != 0:
        return a + c
    elif fix_teen(a) != 0:
        return a
    elif fix_teen(b) != 0:
        return b
    elif fix_teen(c) != 0:
        return c

def fix_teen(n):
    if n in [13, 14, 17, 18, 19]:
        return 0
    else:
        return n

I may be off the mark here as I didn't test my code (no access to a Python environment), but this is the way I'd 'start' to work at it.

def no_teen_sum(a, b, c):
    return fix_teen(a) + fix_teen(b) + fix_teen(c)

def fix_teen(n):
    if n in [13, 14, 17, 18, 19]:
        return 0
    else:
        return n

Quite simple.


Decode ambiguous Morse code Stumbled upon this interview question:

  Given a morse encoded sentence with no spaces or separation between letters or words and a list of words contained in the message, decode it.

This was my solution (and a test case with the jack and jill sentence):

 static String[] alpha = {"a","b","c","d","e","f","g","h","i","j","k",
        "l","m","n","o","p","q","r","s","t","u","v",
        "w","x","y","z"};
static String[] morse = {".-","-...","-.-.","-..",".","..-.","--.","....","..",".---","-.-",".-..","--","-.","---",".--.","--.-",".-.","...","-","..-","...-",
        ".--","-..-","-.--","--.."};

private static HashMap<Character, String> letters = new HashMap<>();

public static void main(String[] args) {
    for (int i = 0; i < alpha.length; i++) {
        letters.put(alpha[i].charAt(0), morse[i]);
    }

    String[] words = {"hill","jack","went","and","jill","the","up"};
    System.out.println(
            decode(".---.--.-.-.-.--.-...---...-...-...--.-.-..-.--.-............-...-..",
                    words)
    );
}

private static String decode(String code, String[] words){
    String[] encodedWords = new String[words.length];
    for (int i = 0; i < words.length; i++) {
        encodedWords[i] = encodeWord(words[i]);
    }

    return decodePossibility(code, words, encodedWords, new StringBuilder());
}

private static String decodePossibility(String code, String[] words, String[] encodedWords, StringBuilder current){
    if(code.length() == 0) return current.toString();
    for (int i = 0; i < encodedWords.length; i++) {
        if(code.length() >= encodedWords[i].length()){
            if(code.substring(0, encodedWords[i].length()).equals(encodedWords[i])){
                String result = decodePossibility(code.substring(encodedWords[i].length()), words, encodedWords, current.append(words[i]).append(' '));
                if(result != null){
                    return result;
                }
            }
        }
    }
    return null;
}

private static String encodeWord(String word){
    StringBuilder stringBuilder = new StringBuilder();
    for (int i = 0; i < word.length(); i++) {
        stringBuilder.append(letters.get(word.charAt(i)));
    }
    return stringBuilder.toString();
}

Any tips on how to improve the code? Any alternative solution?
To back track to a correct solution I would prefer throwing an exception, but sadly I'am not allowed to do so.

White Space

This is hard on my aging eyes:

static String[] morse = {".-","-...","-.-.","-..",".","..-.","--.","....","..",".---","-.-",".-..","--","-.","---",".--.","--.-",".-.","...","-","..-","...-",
    ".--","-..-","-.--","--.."};

This is much easier to read:

static String[] morse = {
    ".-",   "-...",  "-.-.",  "-..",   ".",     "..-.",  "--.",   "....",  "..",   ".---",
    "-.-",  ".-..",  "--",    "-.",    "---",   ".--.",  "--.-",  ".-.",   "...",  "-",
    "..-",  "...-",  ".--",   "-..-",  "-.--",  "--.."
};

alpha[i].charAt(0)

In the following, you use create 26 strings, and call .charAt(0) on each:

static String[] alpha = {"a","b","c","d","e","f","g","h","i","j","k",
    "l","m","n","o","p","q","r","s","t","u","v",
    "w","x","y","z"};
static String[] morse = {".-","-...","-.-.","-..",".","..-.","--.","....","..",".---","-.-",".-..","--","-.","---",".--.","--.-",".-.","...","-","..-","...-",
    ".--","-..-","-.--","--.."};

private static HashMap<Character, String> letters = new HashMap<>();

public static void main(String[] args) {
    for (int i = 0; i < alpha.length; i++) {
        letters.put(alpha[i].charAt(0), morse[i]);
    }

You could simply use one 26-character string:

final static String alpha = "abcdefghijklmnopqrstuvwxyz";

...

for (int i = 0; i < alpha.length(); i++) {
    letters.put(alpha.charAt(i), morse[i]);
}

Or you could statically initialize the structure directly:

Map<Character, String> letters = Map.of(
    'a', ".-",    'b', "-...",  'c', "-.-.",  'd', "-..",  'e', ".",
    'f', "..-.",  'g', "--.",   'h', "....",  'i', "..",   'j', ".---",
    ... etc ...);

Stream API

The following code:

private static String encodeWord(String word){
    StringBuilder stringBuilder = new StringBuilder();
    for (int i = 0; i < word.length(); i++) {
        stringBuilder.append(letters.get(word.charAt(i)));
    }
    return stringBuilder.toString();
}

is pretty straight forward.  It could be reduced to 1 line of code with the Stream API, if you are familiar with it:

private static String encodeWord(String word){
    return word.chars().mapToObj(i -> letters.get((char) i).collect(Collectors.joining());
}

Words & Encoded Words

Right now, you are allocating a parallel array for encodedWords, and matching words and encoded words by index.  You could merge these two structures into one object, a Map<String, String> where encoded words map to their decoded values.  Passing this object to decodePossibility would be more cohesive than two arrays, which could be of different length.

Efficiency

Continuing with the Map<String, String> of decodings...

In decodePossibility, you are looping over all encodedWords trying each one to see if it is the next possibility in the message.  With 7 words, this isn't too inefficient, but if the list of words was larger, it could be.

Instead, you could find the length of the shortest encoded word, and develop a Map of lists of words starting with a prefixes of that length:

Map<String, Map<String, String>> partitioned_decoding;

For instance, the encoding of "the" produces the shortest sequence -..... of 6 characters, so in partition_decoding each key would be the first 6 characters of encodedWords, and each value would contain a mapping of encodedWords to words for only the encodedWords starting with that key.

"-....."  ->  Map.of("-.....", "the")
".---.-"  ->  Map.of(".---.--.-.-.-", "jack")
".---.."  ->  Map.of(".---...-...-..", "jill")
".--.-."  ->  Map.of(".--.-.-", "went", ".--.-..", "and")
"..-.--"  ->  Map.of("..-.--.", "up")
"......"  ->  Map.of(".......-...-..", "hill")

During decodePossibility, if code was at least 6 characters long, you would fetch the first 6 characters of code, look up that entry in partitioned_decoding, and if present, loop over that (ideally only 1) entry in that submap.  In the case of ".--.-.", you'd have to try both "went" and "and", but that is only 2 words, which is still much smaller than examining all 7.

Neither "e" or "t" are valid single letter words, so the worst case would be if words contained the word "a", which would cause the key length to reduce to 2 characters, and words would be partitioned into 4 different bins: "--", ".-", "-.", and "..", which should still provide a 4 times speed-up, since only one quarter of entries would be checked at each stage.

Alternative to Recursion

Instead of recursion, you could use a loop, and allocate your own stacks for current position and word iteration for each level.  When a solution is found, the return statement would directly return the solution - no need to check for a sub-step returning non-null and returning that value, eventually unwinding the call-stack to the top.  When a dead-end is found, you pop the previous state from your own stacks, and continue looping until a solution is found or no solution is possible.


Populating dropdown lists, using SQL vs. using expression trees and reflection I'm working with a large enterprise SQL Server database with dozens of tables that are used mainly as lookups to populate dropdown lists, etc. These tables all follow the convention of having (table)Name as the string value of the item. For example the State table has a StateName column with values like California, Florida, etc.

There is an old method used to load data for a dropdown that uses raw SQL, it looks like this:

public async Task<IEnumerable<string>> GetLookupOptions(string table)
{
    List<string> values = new List<string>();

    using (var command = _context.Database.GetDbConnection().CreateCommand())
    {
        command.CommandText = $"SELECT {table}Name FROM {table} WHERE IsApproved = 1";

        _context.Database.OpenConnection();

        using (var result = await command.ExecuteReaderAsync())
        {
            do
            {
                while (result.Read())
                {
                    values.Add(result.GetString(0));
                }
            } while (result.NextResult());
        }
    }

    return values;
}

This isn't testable using InMemoryDatabase and only works against an actual database connection. I have rewritten it using Reflection and Expressions to query the DbContext.DbSet<T> based on the table name provided, like this:

public IEnumerable<string> GetLookupOptions(string table)
{
    // Get the Type for State and then replace its name with whatever entity this is querying.
    // This is hacky but it's used so the AssemblyQualifiedName will always have correct
    // version info.
    Type t = typeof(State);
    t = Type.GetType(t.AssemblyQualifiedName.Replace("State", table));

    // Get lambda used to filter <table> where IsApproved is true.
    object whereLamda = this.GetType()
        .GetMethod(nameof(CreateWhereExpression), BindingFlags.NonPublic | BindingFlags.Instance)
        .MakeGenericMethod(t)
        .Invoke(this, new object[0]);

    // Get lambda used to select <table>Name from <table>.
    object selectLamda = this.GetType()
        .GetMethod(nameof(CreateSelectExpression), BindingFlags.NonPublic | BindingFlags.Instance)
        .MakeGenericMethod(t)
        .Invoke(this, new object[] { table });

    // Get the DbSet<T> for the <table>.
    object set = _context.GetType()
        .GetMethod("Set")
        .MakeGenericMethod(t)
        .Invoke(_context, new object [0]);

    IEnumerable<MethodInfo> whereMethods = typeof(Enumerable)
        .GetMethods().Where(m => m.Name == nameof(Enumerable.Where));

    // Apply Where() method to DbSet.
    object filteredApproved = whereMethods
        .ElementAt(0)
        .MakeGenericMethod(t)
        .Invoke(set, new object[] { set, whereLamda });

    IEnumerable<MethodInfo> selectMethods = typeof(Enumerable)
        .GetMethods().Where(m => m.Name == nameof(Enumerable.Select));

    // Apply Select() method to filtered query.
    object r = selectMethods
        .ElementAt(0)
        .MakeGenericMethod(t, typeof(string))
        .Invoke(filteredApproved, new object[] { filteredApproved, selectLamda });

    return r as IEnumerable<string>;
}

private Func<T, string> CreateSelectExpression<T>(string tableName)
{
    var param = Expression.Parameter(typeof(T));
    var expr = Expression.Property(param, $"{tableName}Name");

    return Expression.Lambda<Func<T, string>>(expr, param).Compile();
}

private Func<T, bool> CreateWhereExpression<T>()
{
    var param = Expression.Parameter(typeof(T));
    var expr = Expression.Equal(
        Expression.Property(param, "IsApproved"),
        Expression.Constant(true));

    return Expression.Lambda<Func<T, bool>>(expr, param).Compile();
}

My concerns are:

Getting the correct Where and Select methods from Reflection, I don't like relying on ElementAt(). Both methods have overloads with 2 parameters.
Any and all other problems. Have I created a monster?

I'm going to start off by saying I didn't test this code.  There might be some issues but I didn't setup EF and an In Memory Provider.

First I would load all the types right away and cache them and do some filtering to make sure they have the two properties we care about.

You didn't save what your class name is so I'm going with DropDownLookUp

public class DropDownLookUp
{

    private DbContext _context;

    private readonly static IDictionary<string, Type> Mappings;
    private readonly static MethodInfo WhereMethod;
    private readonly static MethodInfo SelectMethod;

    static DropDownLookUp()
    {
        // Load up all the types and cache them making sure they have the properties we need
        Mappings = typeof(State).Assembly.GetLoadableTypes()
            .Where(x => x.GetProperty("IsApproved")?.PropertyType == typeof(bool) &&
                   x.GetProperty(x.Name + "Name")?.PropertyType == typeof(string))
                   .ToDictionary(x => x.Name, x => x);

        Func<IQueryable<object>, Expression<Func<object, bool>>, IQueryable<object>> whereMethod = Queryable.Where;
        WhereMethod = whereMethod.Method.GetGenericMethodDefinition();

        Func<IQueryable<object>, Expression<Func<object, object>>, IQueryable<object>> selectMethod = Queryable.Select;
        SelectMethod = selectMethod.Method.GetGenericMethodDefinition();
    }

We do a one time load of the LoadableTypes and have a type safe way to grab the method infos for the Queryable Select and Where clauses.

Type load I got from StackOverFlow it's like this

public static class AssemblyExtensions
{
    public static IEnumerable<Type> GetLoadableTypes(this Assembly assembly)
    {
        if (assembly == null)
        {
            throw new ArgumentNullException("assembly");
        }
        try
        {
            return assembly.GetTypes().Where(x => x.IsPublic);
        }
        catch (ReflectionTypeLoadException e)
        {

            return e.Types.Where(t => t != null && t.IsPublic);
        }
    }
}

Now we don't want to drop down into IEnumerable until we have to.  We want to stay in ExpressionTrees and build up an IQueryable.

private Expression CreateSelectExpression(Expression source)
{
    var param = Expression.Parameter(source.Type);
    var project = Expression.Property(param, $"{source.Type.Name}Name");
    var selector = Expression.Lambda(typeof(Func<,>).MakeGenericType(source.Type, typeof(string)), project, param);
    return Expression.Call(SelectMethod.MakeGenericMethod(source.Type, typeof(string)), source, selector);
}

private Expression CreateWhereExpression(Expression source)
{
    var param = Expression.Parameter(source.Type);
    var filter = Expression.Equal(
        Expression.Property(param, "IsApproved"),
        Expression.Constant(true));

    var whereClause = Expression.Lambda(typeof(Func<,>).MakeGenericType(source.Type, typeof(bool)), filter, param);
    return Expression.Call(WhereMethod.MakeGenericMethod(source.Type), source, whereClause);
}

These methods now call the Queryable.Select and Queryable.Where.    If you want you could cache these methods per Type but I find building Expressions isn't that taxing, where compiling them is, but it's worth testing to see if you want to add some cache to these methods.

For the main method we can just check the Mappings and if not found either return empty enumerable or throw.  We can still use the reflection on Set. If you really wanted you could build an expression tree to give you back the IQueryable when passing in a type.  I would base that on if you have performance issues or not.

public async Task<IEnumerable<string>> GetLookupOptions(string table)
{
    Type type;
    if (!Mappings.TryGetValue(table, out type))
    {
        // Or you can throw your choice
        return Enumerable.Empty<string>();
    }

    // Get the DbSet<T> for the <table>.
    IQueryable set = (IQueryable)_context.GetType()
        .GetMethod("Set")
        .MakeGenericMethod(type)
        .Invoke(_context, new object[0]);

    var query = set.Provider.CreateQuery<string>(CreateSelectExpression(CreateWhereExpression(set.Expression)));
    return await query.ToListAsync().ConfigureAwait(false);
}

We can use the IQueryable Provider to create a new Query based on the expressions we build - which should execute the where and select up on the server and we can now use the ToListAsync to make the method async again.


Logging the caller ip in a asmx web service using NLog private void log(MethodExecutionArgs args, string message, LogLevel level)
{
    var calling_ip = ((System.Web.Services.WebService)args.Instance).Context.Request.UserHostAddress;
    string method = args.Method.Name;
    LogEventInfo logInfo = new LogEventInfo(level, logger.Name, message);
    logInfo.Properties.Add("method", method);
    logInfo.Properties.Add("calling_ip", calling_ip);
    logger.Log(logInfo);
}

I have written this method to log the ip address of the caller. The calling_ip value is accessed in the NLog.config via the event_context ${event-context:item=calling_ip}

Can this code be improved?

One possible change would be to use object initializer to set up the properties:

var logInfo = new LogEventInfo(level, logger.Name, message)
{
    Properties = { { "method", method }, { "calling_ip", calling_ip } }
};

But I'm not sure this actually improves the code much, especially since using this syntax this way is uncommon, so it can be confusing.


Memoizing a tree's parent pointers in Python I have a simple binary tree that has no parent pointers.
class Node:
    def __init__(self, val=0, left=None, right=None):
        self.val = val
        self.left = left
        self.right = right

class Tree:
    def __init__(self, root=None):
        self.root = root

I need to traverse the tree from bottom up and cannot modify the Node class, so I'd like to memoize the parents.  I can modify the Tree class as such:
class Tree:
    def __init__(self, root=None):
        self.root = root
        self.memo = {}

    def memoize(self, node, parent_node):
        if node is None:
            return False
        self.memo[id(node)] = parent_node
        self.memoize(node.left, node)
        self.memoize(node.right, node)

    def get_parent(self, child):
        return self.memo[id(child)]

If I create a tree, memoize it, and run get_parent() I see what I expect:
a = Node(1)
tree = Tree(a)
b = Node(2)
c = Node(3)
a.left = b
a.right = c

tree.memoize(a, None)   # Tree root is instantiated with no parent
parent = tree.get_parent(b)

print(tree.memo)
>>> {4405793712: None, 4405793856: <__main__.Node instance at 0x1069b13b0>, 4405793928: <__main__.Node instance at 0x1069b13b0>}

print(parent.val)
>>> 1

This seems to work nicely.  However, I am a Python beginner, and want to know: is there a more Pythonic way to do this?

Nodes are usually used internally within the tree and are created inside the insert function of your tree. This is to hide the implementation for users as well as prevent any corruptions that could occur due to external mutability. This isn't Python specific, but more about data structures in general. Data is encapsulated in the structure and use specific operations (functions) for retrieval/mutations.
I'm not sure what your use case is here, but I'd deter you from allowing Nodes to be used outside of the Tree class (if possible).
To traverse the tree bottom up, here are two methods you can try:

Reverse level order traversal

implement the binary tree with a list internally. Then if you wanted to traverse from the bottom up, the last item in the list would certainly be the node at the bottom of the tree and you can get its parent via a list index (you can work this out by reading the previous link).


Given two strings, determine if one is a permutation of the other After comparing my newbie attempt with other people's answers, I have a couple questions:

I've made s1_set the size of s1. I've seen other people assume that the strings are ASCII-256 and use s1_set(256) but to me this doesn't make sense. Won't s1_set have at most s1.size() bytes (and fewer if s1 has duplicate characters)?
I've also seen explicit casting before accessing s1_set: int val = static_cast<int>(x);. Is this necessary? Is it bad form to use implicit casting?
Would it be preferred to use the regular for statement, or does the range-for work just as well?

#include <iostream>
#include <vector>

using namespace std;

bool is_permutation(const string& s1, const string& s2) {
  if (s1.size() != s2.size()) {
    return false;
  }

  vector<int> s1_set(s1.size());
  for (const auto& x : s1) {
    ++s1_set[x];
  }

  for (const auto& x : s2) {
    if (--s1_set[x] < 0) {
      return false;
    }
  }

  return true;
}

int main(int argc, char* argv[]) {
  cout << is_permutation("AABC", "BCBA") << endl; // false
  cout << is_permutation("BC", "CBA") << endl; // false, different sizes
  cout << is_permutation("ABC", "CBA") << endl; // true
  cout << is_permutation("AABC", "ACBA") << endl; // true
}

This code is broken.

// This creates an array with size elements all set to zero.
// But your string (in the examples) are not more than 4 long.
// So the largest vector is 4 elements long.
vector<int> s1_set(s1.size());

// Here x is char.
for (const auto& x : s1) {
// It is undefined if char is signed or unsigned.
// so the range of values for x is
//       0 -> 255
// or -127 -> 128
//
// Either way the range is well beyond the size of
// the array defined above.


Is this password hashing function safe? Would this be secure enough to hash passwords?

/**
 * Creates a SHA-256 hash
 * @param  string $salt Salt for hashing algorithm
 * @param  string $string What is getting hashed
 * @return string Returns a hashed string
 */
public function hash ($string, $salt) {
    $crypt = crypt($string, '$5$rounds=5000$' . $salt . '$');
    $hash = str_replace('$5$rounds=5000$' . $salt . '$', '', $crypt);

    return $hash;
}

Using SHA-256 with a salt is not that bad. But PHP has built in functions like password_hash and password_verify to hash passwords. It uses bcrypt, which is currently one of the best methods to hash passwords these days. I would recommend this approach and it is just as easy to use like the function you've written.


Implement code coverage flush function with boiler plate code In each shared library that I build, it need to call __gcov_flush but that call should only be made when building the instrumented binaries for code review.
In that case I'm passing

-D__CODE_COVERAGE__

option as a compilation flag.

And in each .so file.

#ifdef __CODE_COVERAGE__

void cscomm_gcov_flush()
{
    __gcov_flush();
}
#endif // __CODE_COVERAGE__

So I'm keeping the same source tree for both code coverage , debug and release.
Is this approach is oky, good or bad ? Can you review?

This works (at least I assume it does), but it's not how I'd do things.

I'd put the code:

void cscomm_gcov_flush()
{
    __gcov_flush();
}

...into a file by itself (e.g., flusher.c or flusher.cc, as you prefer). Then I'd have the build tool decide whether to compile and link that into the .so files depending on whether it's building an instrumented binary or not.

OBJS = // file list here

ifdef __CODE_COVERAGE__
    OBJS += flusher.o
endif

Syntax varies with the build tool you're using of course, but I think the general idea is fairly apparent.

Looking at the code more generally, names that start with an underscore followed by another underscore or a capital letter (including __CODE_COVERAGE__) are all reserved for the implementation, so defining them leads to undefined behavior. If the compiler does it in response to a command line argument (or similar) that's fine, but if you're doing it yourself, you have undefined behavior.


AlwaysUpdate attribute for Entity Framework Code First POCO The entity framework will only save properties it detects has changed via its proxy classes.  I have a situation where I want a property to always be saved no matter if it changed or not.

I wrote a blank attribute called AlwaysUpdate which I apply to the property.  I then overrode SaveChanges to scan for these attributes and mark the field as Modified.  Is this code all right?

public override int SaveChanges()
{
    var changeSet = ChangeTracker.Entries();
    if (changeSet != null)
    {
        foreach (var entry in changeSet.Where( c=> c.State == System.Data.EntityState.Modified ))
        {
            foreach (var prop in entry.Entity.GetType().GetProperties().Where(p => p.GetCustomAttributes(typeof(AlwaysUpdateAttribute), true).Count() > 0).Select( p => p.Name ))
            {
                if (entry.State == System.Data.EntityState.Added || entry.State == System.Data.EntityState.Unchanged) continue;
                // Try catch removes the errors for detached and unchanged items
                try
                {
                    entry.Property(prop).IsModified = true;
                }
                catch { }
            }
        }
    }
    return base.SaveChanges();
}

Instead of checking .Count() > 0 which needs to iterate over the IEnumerable you can just use .Any() which only checks if at least one item is contained in the IEnumerable
you should extract the result of typeof(AlwaysUpdateAttribute) to a variable.

Type desiredType = typeof(AlwaysUpdateAttribute);

you can restrict the outer loop by adding the "continue condition" to the Where() method. For readability we will do this outside of the loop

changeSet = changeSet.Where( c=>
            c.State == System.Data.EntityState.Modified
              && !(entry.State == System.Data.EntityState.Added
              || entry.State == System.Data.EntityState.Unchanged)
             );

you shouldn't shorten any variable names -> prop should be propertyName

Refactoring

Implementing all above will lead to

public override int SaveChanges()
{
    var changeSet = ChangeTracker.Entries();
    if (changeSet != null)
    {
        changeSet = changeSet.Where( c=>
                 c.State == System.Data.EntityState.Modified
                  && !(entry.State == System.Data.EntityState.Added
                  || entry.State == System.Data.EntityState.Unchanged)
                  );

        Type desiredType = typeof(AlwaysUpdateAttribute);

        foreach (var entry in changeSet)
        {
            foreach (var propertyName in entry.Entity.GetType().GetProperties().Where(p => p.GetCustomAttributes(desiredType , true).Any()).Select( p => p.Name ))
            {
                // Try catch removes the errors for detached and unchanged items
                try
                {
                    entry.Property(propertyName).IsModified = true;
                }
                catch { }
            }
        }
    }
    return base.SaveChanges();
}

    foreach (var entry in changeSet)
    {
        foreach (var prop in entry.Entity.GetType().GetProperties().Where(p => p.GetCustomAttributes(desiredType , true).Any()).Select( p => p.Name ))
        {
            // Try catch removes the errors for detached and unchanged items
            try
            {
                entry.Property(prop).IsModified = true;
            }
            catch { }
        }


Leetcode: Map Sum Pairs C# Implement a MapSum class with insert, and sum methods.

  For the method insert, you'll be given a pair of (string, integer).
  The string represents the key and the integer represents the value. If
  the key already existed, then the original key-value pair will be
  overridden to the new one.

  For the method sum, you'll be given a string representing the prefix,
  and you need to return the sum of all the pairs' value whose key
  starts with the prefix.

Example 1:
Input: insert("apple", 3), Output: Null
Input: sum("ap"), Output: 3
Input: insert("app", 2), Output: Null
Input: sum("ap"), Output: 5

Please comment on performance and style

using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TrieQuestions
{
    [TestClass]
    public class TrieMapSum
    {
        [TestMethod]
        public void MapSumTest()
        {
            MapSum mapSum = new MapSum();
            mapSum.Insert("apple", 3);
            Assert.AreEqual(3, mapSum.Sum("ap"));
            mapSum.Insert("app", 2);
            Assert.AreEqual(5, mapSum.Sum("ap"));
        }
    }

    public class MapSum
    {
        private TrieMSNode Head;
        /** Initialize your data structure here. */
        public MapSum()
        {
            Head = new TrieMSNode();
        }

        public void Insert(string key, int val)
        {
            var current = Head;
            foreach (var letter in key)
            {
                if (!current.Edges.ContainsKey(letter))
                {
                    current.Edges.Add(letter, new TrieMSNode());
                }
                current = current.Edges[letter];
            }

            current.IsTerminal = true;
            current.Value = val;
        }

        public int Sum(string prefix)
        {
            int sum = 0;
            var current = Head;
            foreach (var letter in prefix)
            {
                if (!current.Edges.ContainsKey(letter))
                {
                    return sum;
                }

                current = current.Edges[letter];
            }
            // we use dfs for each edges of the trie;
            return DFS(current);
        }

        private int DFS(TrieMSNode current)
        {
            if (current == null)
            {
                return 0;
            }

            int sum = current.IsTerminal ? current.Value : 0;
            foreach (var edge in current.Edges.Values)
            {
                sum += DFS(edge);
            }
            return sum;
        }
    }

    internal class TrieMSNode
    {
        public Dictionary<char, TrieMSNode> Edges { get; set; }
        public bool IsTerminal;
        public int Value;

        public TrieMSNode()
        {
            Edges = new Dictionary<char, TrieMSNode>();
            IsTerminal = false;
            Value = 0;
        }
    }
}

In Insert(...) you should use TryGetValue instead of ContainsKeyfor efficiency:

  foreach (var letter in key)
  {
    if (!current.Edges.TryGetValue(letter, out var edge))
    {
      edge = current.Edges[letter] = new TrieMSNode();
    }
    current = edge;
  }

Name your methods after what they do, not after their implementation:

  DFS(...)

could be GetSumFrom(...)

public int Sum(string prefix)
{
  int sum = 0;
  var current = Head;
  foreach (var letter in prefix)
  {
    if (!current.Edges.ContainsKey(letter))
    {
      return sum;
    }
    current = current.Edges[letter];
  }

Here the sum variable is redundant because it is never changed so you could just return 0 from the loop

Again the loop can be optimized:

  foreach (char letter in prefix)
  {
    if (!current.Edges.TryGetValue(letter, out current))
      return 0;
  }

DFS()

can be simplified using LINQ:

    private int GetSumFrom(TrieMSNode current)
{
  if (current == null)
  {
    return 0;
  }

  return current.Edges.Aggregate(current.Value, (sum, kvp) => sum + GetSumFrom(kvp.Value));
}

You really don't have to check for IsTerminal because you know that only terminal nodes have a values different from 0.

You should test this LINQ-approach against your own plain foreach-loop, and you'll maybe find the latter fastest.


Thread synchronization and happens-before relation in simple examples I wrote those examples to show different approaches to sharing results between threads. The whole code is on GitHub. All examples should return "Hello world".

Are these OK from the perspective of data visibility?

The most hardcore one:

import com.google.common.util.concurrent.Uninterruptibles;

import java.util.concurrent.TimeUnit;
public class WaitNotifyExample {

    static String result1;
    static String result2;

    private static final Object object = new Object();

    public static void main(String[] args) {

        Thread producer1 = new Thread() {
            @Override
            public void run() {
                Uninterruptibles.sleepUninterruptibly(200, TimeUnit.MILLISECONDS);
                result1 = "Hello";
                synchronized (object) {
                    object.notify();
                }
            }
        };

        Thread producer2 = new Thread() {
            @Override
            public void run() {
                Uninterruptibles.sleepUninterruptibly(400, TimeUnit.MILLISECONDS);
                result2 = "world";
                synchronized (object) {
                    object.notify();
                }
            }
        };

        producer1.start();
        producer2.start();

        synchronized (object) {
            while(true) {
                try {
                    object.wait();
                } catch (InterruptedException e) {
                    System.out.println("Interrupted!");
                }

                System.out.println("Notified!");

                if(result1 != null && result2 != null) {
                    System.out.println(result1 + " " + result2);
                    break;
                } else {
                    System.out.println("Not ready");
                }
            }
        }

    }
}

Your code here is pretty mixed up and is not representative of a decent test of concurrency. It will work and give you the results you expect, but the reason is a bit polluted.

The internal code for System.out.println() looks like:

public void println(String x) {
    synchronized (this) {
        print(x);
        newLine();
    }
}

As a result, there are many more synchronizations than you expect. These are nested inside another synchronization block.

In essense, any time there is a println() you have a possibly conflicting synchronization problem.

In this case, it does not make much of a difference, but your code concerns me.

Additionally, it is traditional to incorporate the wait-conditions in the while-loop, and not as a break condition. Your code:

   synchronized (object) {
        while(true) {
            try {
                object.wait();
            } catch (InterruptedException e) {
                System.out.println("Interrupted!");
            }

            System.out.println("Notified!");

            if(result1 != null && result2 != null) {
                System.out.println(result1 + " " + result2);
                break;
            } else {
                System.out.println("Not ready");
            }
        }
    }

Would be better as:

   synchronized (object) {
        while(result1 == null && result2 == null) {
            object.wait();
        }
    }
    System.out.println(result1 + " " + result2);

Note that I have stripped out your InterruptException handling. It was not sufficient for "real" code, and if you replace it, should probably go outside the calls. If someone interrupts your code, continuing in a loop is probably not the right thing to do.


Checking whether a number is a power of 10 Is there a better way to check whether a number is a power of 10? This is what I've implemented:

    public class PowersOfTen {

    public static boolean isPowerOfTen(long input) {

        if (input % 10 != 0 || input == 0) {
            return false;
        }

        if (input == 10) {
            return true;
        }

        return isPowerOfTen(input/10);
    }

    public static void main(String[] args) {

        System.out.println("1000: " + isPowerOfTen(1000));
        System.out.println("4: " + isPowerOfTen(4));
        System.out.println("0: " + isPowerOfTen(0));
        System.out.println("10: " + isPowerOfTen(10));
        System.out.println("100: " + isPowerOfTen(100));
    }
}

Recursing seems pretty heavy for this. You can use much the same logic but with a simple loop:
while (input >= 10 && input % 10 == 0)
  input /= 10;
return input == 1;

This checks to make sure the input is at least 10 (since any integers below that, including zero and negative values, can't be powers of 10) and also a multiple of 10 (as all powers of 10 greater than 1 obviously are). It does the simpler comparison first as a small micro-optimization; the short-circuit evaluation will avoid performing an unnecessary division to compute an unneeded modulus.
If both conditions are met it does an integer division by 10 and repeats the process. For any power of 10, this will terminate with the value 1; if 1 itself is passed in, it will do so without ever executing the body of the while loop.


Counting pairs that have a given difference in Java You will be given an integer k and a list of integers. Count the number of distinct valid pair of integers (a,b) in the list for which a+k=b.

  For example, the array [1,1,1,2] has two different valid pairs:(1,1) and (1,2).
  Note that the three possible instances od pair (1,1) count as a single valid pair, as do the three possible instances of the pair (1,2). if k=1, then this means we have total of one 1 valid pair which satisfies a+k=b=>1+1=2, the pair (1,2).

My code:

public class PairArrayStream {

    public static void main(String[] args) {
        int k =1;
        List<Integer> input = Arrays.asList(1,1,1,2);
        HashSet<HashSet> hs = new HashSet<HashSet>();
        IntStream.range(0,  input.size())
        .forEach(i -> IntStream.range(0,  input.size())
            .filter(j -> i != j && input.get(i) - input.get(j) == k)
            .forEach(j -> {
                 HashSet inner = new HashSet<>();
                 inner.add(input.get(j));
                 inner.add(input.get(i));
                 hs.add(inner);
            })
    );
        System.out.println("OutPut "+hs.size());
}

}

Without java 8 features::

int k =1;
  List<Integer> input = Arrays.asList(1,1,1,2);
  HashSet<HashSet> hs = new HashSet<HashSet>();
   for(int i =0 ; i<numbers.size();i++){
       for(int j = i; j<numbers.size();j++){
           if(Math.abs(numbers.get(j)-numbers.get(i)) == k){
                HashSet inner = new HashSet<>();
                 inner.add(numbers.get(j));
                 inner.add(numbers.get(i));
                 hs.add(inner);
           }
       }
   }

Well, I am getting the correct output but 40% of the test cases gives me a timeout. Opinions and tactics are welcomed to make the code better and fast.

From a review of your code, I see that you loop over your input in a nested for loop. This is actually not required to solve the problem. Instead, create one hashset for every number in your numbers array, and another hashset for every number in numbers + k. Then we only have to check for intersections.

This should take your \$O(n^2)\$ algorithm and turn it into a nicer \$O(n)\$. Here's my suggested code (note that I included an inner class to store the pairs, but you could use builtins to solve it too:

private static class Pair {
    public int a;
    public int b;
    public Pair(int a, int b) {
        this.a = a;
        this.b = b;
    }
    public String toString() {
        return "("+a+","+b+")";
    }
}

public static List<Pair> getPairsFast(int k, List<Integer> numbers) {
    HashSet<Integer> hLow = new HashSet<>();
    HashSet<Integer> hHigh = new HashSet<>();
    List<Pair> ret = new ArrayList<>();
    for (int i : numbers) {
        hLow.add(i);
        hHigh.add(i+k);
    }
    for (int i : hHigh) {
        if (hLow.contains(i)) {
            ret.add(new Pair(i-k, i));
        }
    }
    return ret;
}

From some testing, it indeed seems to perform better the larger the input is, and it is >400 times faster for an input of size \$10^5\$.


Java MVC Main Method and Controller I'm coding a school project that involves MVC and threading.

This is my Main class. From there I launch the actual client.

public class Main {

    public static void main(String[] args) {

        Controller myController = new Controller();

        ScreenModel screenModel = new ScreenModel();

        ChessBoardModel chessBoardModel = new ChessBoardModel();

        ChessBoard chessBoardView = new ChessBoard(myController);

        View myView = new View(myController, chessBoardView);

        chessBoardModel.addObserver(chessBoardView);

        screenModel.addObserver(myView);

        myController.addModel(screenModel);

        myController.addChessBoardModel(chessBoardModel);

        myController.addView(myView);

    }

}

Is this too messy? I essentially have one model, the ScreenModel, that handles moving back and forth between screens and another model which handles the Chess Board. Here's my controller:

public class Controller {

    ScreenModel model;
    View view;
    User user = new User();
    ClientSocketManager csm;
    ChessBoardModel chessBoardModel;

    /**
     * Called when the user logs in.
     */

    public Controller() {

        csm  = new ClientSocketManager();

        csm.connect("localhost", 4444);

    }

    public void login() {

        //Create userdata object upon login.

        model.handleLogin();

    }

    /**
     * Called when the user creates a game (from the main menu or the join game/games in progress screens)
     */

    public void createGame() {

        model.handleCreateGame();

    }

    /**
     * Called when the user clicks on the Join Game button in the main menu.
     */

    public void joinGameButton() {

        model.handleJoinGameButton();

    }

    /**
     * Called when the user chooses a game clicks on the "join" button.
     */

    public void joinGame() {

        model.handleJoinGame();

    }

    /**
     * Called when the user clicks on the "My Games in Progress" button.
     */

    public void myGamesInProgress() {

        model.handleMyGamesInProgress();

    }

    /**
     * Called when user cancels joining a new game or joining a game in progress.
     */

    public void cancel() {

        model.handleCancel();

    }

    /**
     * Handles making a move from the chess board.
     * @param from   The non-empty from square.
     * @param to     The non-empty to square.
     */

    public void makeMove(Cell from, Cell to) {

        chessBoardModel.handleMakeMove(from,to);

    }

    /**
     * Methods related to the Chess Board game in progress
     */

    /**
     * Called when the user clicks on the "back" button inside the Chess Board screen.
     */

    public void back() {

        model.handleBack();

    }

    /**
     * Chess Board Screen methods below
     */

    public void offerDraw() {

        chessBoardModel.handleOfferDraw();

    }

    public void resign() {

        chessBoardModel.handleResign();

    }

    /**
     * Handles flipping the board from the Chess Board Screen
     */

    public void flipBoard() {

        chessBoardModel.handleFlipBoard();

    }

    /**
     * Methods for adding models and views.
     * @param m
     */

    public void addModel(ScreenModel m){

        this.model = m;
        m.setUser(user);

    }

    public void addChessBoardModel(ChessBoardModel c) {

        this.chessBoardModel = c;
        c.setUser(user);

    }

    public void addView(View v){

        this.view = v;

    }

}

Is the controller the appropriate place for the ClientSocketManager, or should it be placed inside the model? What do you think of the rest of my controller?

Just looking at you Main class, you notice some strange things.

Passing an already known instance:

ChessBoard chessBoardView = new ChessBoard(myController);
View myView = new View(myController, chessBoardView);

The ChessBoard holds (normally) an instance of Controller. The View hold an instance of ChessBoard who holds the instance of Controller. So passing the controller to View is not needed, if you need the controller in the View just call chessboard.getController();.

Beans who have each other instances:

Controller myController = new Controller();
View myView = new View(myController, chessBoardView);
myController.addView(myView);

It's a strange setup, but if used for the Observer-observable pattern I can see your logic. But still, it's at the wrong place. If you need this, pass the instance of view to the controller in the constructor of view.

public View (ChessBoard board) {
    this.board = board;
    board.getController().addView(this);
}

Attention, you will have a leaking constructor warning with this.

Method naming:

I see you use in the Controller addModel, addView,... When you use addXxx this should mean that you add this to a Collection. If it's not adding to a Collection use setXxx so everyone who reads your code knows it.

Why is this important:

With set you know you lose the previous instance.
With add you normally hold your previous instance.

MVC:

Your pattern is almost correct except the fact that your View holds ChessBoard, as I can see from the name of the variable is also a view. If your ChessBoardModel is changed, it should notify the View itself and not the ChessBoard itself.


Counting pairs of integers which have a predefined difference I'm doing this HackerRank problem:

  Given \$N\$ integers, count the number of pairs of integers whose difference is \$K\$.

So far, I've ended up with this code:

def countPairs(ar, n, k):
    ar.sort()
    temp = list(ar)
    i = 0
    j = n - 1
    cnt = 0
    while i < n:
        while abs(ar[i] - temp[j]) > k and j > 0:
            j -= 1
        if abs(ar[i] - temp[j]) == k:
            cnt += 1
        i += 1
        j = n - 1
    return cnt

if __name__ =='__main__':
    N, K = map(int, raw_input().strip().split())
    A = map(int, raw_input().strip().split())
    print countPairs(A, N, K)

The problem is some test cases terminated due to timeout (took more than 10 seconds).

I tried calculating the time complexity. Sorting the list in the first step of countPairs takes \$O(n\log n)\$ and copying the list in the second step into temp takes \$O(n)\$.

This link says that the "two-pointer technique" has \$O(n)\$ complexity. So, the overall complexity is \$O(n \log n)\$. Does anyone know how to make it run faster?

To be honest, I think the "two-pointer technique" isn't \$O(n)\$, but \$O(n^2)\$ because of the two while loops, but I'm not sure.

Style

1-letter variable names doesn't have meaning. It is acceptable for i, j or k as loop indexes but other than that it mostly harms readability;
Same for temp or abreviations (ar, cnt, diff): give these variables some real names;
functions names should be snake_case instead of camelCase;
iterating over the values of a sequence is more easily done using a for loop:

for value in ar:
    # do something with `value` instead of `ar[i]`

Since you’re not modifying it, you can remove temp and access ar[j] directly.
You don't need to strip() before a split() without arguments, it will be done automatically.
You correctly used if __name__ == '__main__', that's a good habit to keep.

After that, the code could look like:

def count_pairs(data, length, delta):
    data.sort()
    i = length - 1
    num_pairs = 0
    for value in data:
        while abs(value - data[i]) > delta and i > 0:
            i -= 1
        if abs(value - data[i]) == delta:
            num_pairs += 1
        i = length - 1
    return num_pairs

if __name__ =='__main__':
    N, K = map(int, raw_input().split())
    data = map(int, raw_input().split())
    print count_pairs(data, N, K)

Complexity

For now, you’re iterating over the same list several times to find the pairs and you need to sort the list beforehand for an efficient computation. But you could compute, for each element, the element that need to be present to form a pair (\$O(n)\$) and then, for each initial element (\$O(n)\$), check if it exists in the array of computed "need-to-be-there" elements.

Luckyly, Python's sets can provide such checks with an \$O(1)\$ performace, for an overall \$O(n)\$ time complexity:

def count pairs(data, delta):
    needed_values = set(value + delta for value in data)
    return sum(value in needed_values for value in data)

Here I abuse the fact that booleans are integers after all, but you could write it in a more proper way using:

def count_pairs(data, delta):
    needed_values = set(value + delta for value in data)
    exist_in_pair = [value for value in data if value in needed_values]
    return len(exist_in_pair)


Formatting seconds as hours:minutes:seconds This is basically my school homework and I want to know what to improve, and what not to do.
The homework basically is this:
You will get a input in seconds, which is supposed to be shown as H:MM:SS where H is hours M is minutes and S is seconds. The formatting is supposed to look like in my example. ( note: In output, 0s from the left will be ignored and replaced with spaces )

Input:
3000
730
308
3967
15
1

Output:
The count of entires: 6
  50:00
  12:10
   5:08
1:06:07
     15
      1

So basically, I did a code based on the example. I haven't got more informations, just that but it is enough and fun.
Here goes my code:

#include <iostream>
#include <fstream>

inline int CountSpaces ( int hod, int min, int sec ){
    int spaces = -1;
    // 1:06:07
    if ( hod )
        return 0;
    if ( min >= 10 )
        return 2;
    if ( min < 10 && min > 0 )
        return 3;
    if ( sec >= 10 )
        return 5;
    if ( sec < 10 )
        return 6;
    // in the case something goes wrong..
    return spaces;
}

// convert time to hour,min,seconds
void CalcTime ( int value, int& hour, int& min, int& sec ){
    hour = value / 3600;
    min = value % 3600 / 60;
    sec = value % 60;
}

// output spaces
void Space ( int* hour, int* min, int* sec ){
    int spaces = CountSpaces ( *hour , *min , *sec );
    if ( !spaces ) return;
    for ( int i = 0 ; i < spaces ; i++ ){
        std::cout << ' ';
    }
}

void Write ( int time ){
    if ( time <= 0 )
        return;

    int hour;
    int min;
    int sec;

    CalcTime ( time , hour , min , sec );
    Space ( &hour , &min , &sec );

    bool bHour = false;
    bool bMin = false;
    if ( hour > 0 ){
        bHour = true;
        std::cout << hour << ':';
    }
    if ( min < 60 && min > 0 ){
        bMin = true;
        if ( bHour ){
            if ( min < 10 ){
                std::cout << '0';

            }
        }
        if ( min > 0 )
            std::cout << min << ':';

    }
    if ( sec < 60 && sec >= 0 ){
        if ( sec < 10 && bMin ){
            std::cout << '0';
        }
        std::cout << sec << std::endl;
    }
}

int main ( ){
    std::ifstream fin ( "in.txt" );

    if ( !fin ) return 1;

    int num;
    int count = 0;
    int arr[100];
    while ( fin >> num ){
        arr [ count++ ] = num;
    }
    std::cout << "The count of entires: " << count << std::endl;
    for ( int i = 0 ; i < count ; i++ ){

        Write ( arr [ i ] );
    }

    return 0;
}

Bug
Handling a case of 0 minutes is a bit more involved. For example, giving 3601 as an input produces 1:1 instead of expected 1:00:01.

Trust your math
min and sec are guaranteed to be less than 60. There is no point to test for it.

Computing spaces
I recommend not to compute them explicitly. You know that the output should be right-aligned into the 7-character-wide field. An std::setw(7)  manipulator is specifically designed to do the job. Write your output to the string (or a stringstream), and
      std::cout << std::setw(7) << s << '\n';

will achieve the goal.