The Miller-Rabin Primality Test in Clojure I am new to clojure and I wanted to test out my skills by taking an existing implementation written in Python and writing it in Clojure.

Concerns

My main concerns are where I use cond and put an upto parameter in the function definitions of isWitness and probablyPrime. I am not sure if that's the best way to loop (I tried using the actual loop construct but found out, you couldn't break out of it).

Furthermore, on line 41 where I call the isWitness function from probablyPrime seems very messy. I am calling decompose twice when calling it once would be best (I don't know how to simulate local "variables" or use let properly).

Also I have separated a part of my isWitness into another function called checkWitness. I am not sure if that's the best way to do it.

(ns jeremy-kun.miller-rabin
  (:require [clojure.math.numeric-tower :as math]))

;; This is the clojure version of the original python code done on jeremykun website
;; https://jeremykun.com/2013/06/16/miller-rabin-primality-test/
(defn pow [x y & [z]]
  (if z
    (mod (math/expt x y) z)
    (math/expt x y)))

(defn rand-between [n]
  (rand-nth (range 2 n)))

(defn decompose [exponent-of-two n]
  (if (not= 0 (mod n 2))
    {:exponent exponent-of-two :n n}
    (decompose (+ 1 exponent-of-two) (/ n 2))))

(defn checkWitness [possibleWitness p exponent remainder upto]
  (cond (> upto exponent) true
    (= (- p 1) (pow possibleWitness 2 p)) false
    :else (checkWitness (pow possibleWitness 2 p) p exponent remainder (inc upto)))
  )

(defn isWitness [possibleWitness p exponent remainder upto]
  (cond
    (or (= (pow possibleWitness remainder p) 1) (= (pow possibleWitness remainder p) (- p 1))) false
    (zero? exponent) true
    :else (checkWitness (pow possibleWitness remainder p) p exponent remainder upto)))

(defn probablyPrime [p accuracy upto]
  (if (or (= p 2) (= p 3))
    true)

  (if (< p 2)
    false)

  (cond
    (> upto accuracy) true
    (isWitness (rand-between (- p 2)) p (:exponent (decompose 0 (- p 1))) (:n (decompose 0 (- p 1))) 0) false
    :else (probablyPrime p accuracy (inc upto))
    ))

;;(def test-witness [[10 5] [11 9] [12 5] [13 10] [14 11] [15 3] [16 13] [17 3] [18 7] [19 11]])

;;
;(prn
;  (map (fn [x] (isWitness (last x) (first x)
;                         (:exponent (decompose 0 (- (first x) 1))) (:n (decompose 0 (- (first x) 1)))
;                         0))
;       test-witness))

(prn (probablyPrime 25 100 0))                              ;; false
(prn (probablyPrime 100 100 0))                             ;; false
(prn (probablyPrime 151 100 0))                             ;; true
(prn (probablyPrime 97 100 0))                              ;; true

Test Cases

I have already included four test cases for primes. If you want to test more then just replace the first parameter and leave the rest as is, especially upto.

The block of code that is commented out tests the isWitness function. You can test it out by uncommenting the code and the value test-witness. The value test-witness is a list of lists. Each element in the list of list is the value to check and a random number between 2 and the value you want to test for primality minus 2.

For instance, [10 5], 10 is the value to check for primality and 5 is a random number between 2 and 10.

If you uncomment the block and run the code, it should return

(true false true false true true true false true false)

which is what the python code runs if you use the same values.

There is an error in the translation of the probablyPrime function. In Clojure, a function body is an implicit do form. The two if forms and  the cond form:

(defn probablyPrime [p accuracy upto]
  (if...
    )

  (if ...
    )

  (cond
    ...
    ))

... will be evaluated in succession. The values of the (if ...)s will be discarded; the result of the (cond ...) will be returned. The Python returns the associated value if either if condition is satisfied.

For example, the Clojure gives

(probablyPrime -2 0 1)
;true

... whereas the Python returns false.

I suggest you bring all the conditions under the cond:

(defn probablyPrime [p accuracy upto]
  (cond
    (or (= p 2) (= p 3)) true
    (< p 2) false
    (> upto accuracy) true
    (isWitness (rand-between (- p 2)) p (:exponent (decompose 0 (- p 1))) (:n (decompose 0 (- p 1))) 0) false
    :else (probablyPrime p accuracy (inc upto))))

There are other problems that make your Clojure unworkable for testing large numbers:

You use proper recursion to express Python for loops. This may
cause stack overflow and will certainly slow things down. Use
Clojure's recur instead.
Your rand-between function is \$\mathcal{\Theta}(n)\$. This is a performance
disaster when \$n\$ is huge.

There are also some constructions that would improve your code in small ways:

Use let forms to avoid recalculating expressions.
Use inc and dec instead of (+ ... 1) and (- ... 1).
Prefer explicit arities to rest parameters in function pow.

With these improvements, the rest of the code now looks as follows:

(ns miller-rabin.core
  (:gen-class)
  (:require [clojure.math.numeric-tower :as math]))

(defn pow
  ([x y] (math/expt x y))
  ([x y z] (mod (pow x y) z)))

(defn rand-between [n]
  (+ (rand-int (- n 2)) 2))

(defn decompose [exponent-of-two n]
  (if (odd? n)
    {:exponent exponent-of-two :n n}
    (recur (inc exponent-of-two) (quot n 2))))

(defn checkWitness [possibleWitness p exponent remainder upto]
  (let [possibleWitness (pow possibleWitness 2 p)]
    (or
     (> upto exponent)
     (and
       (not= (dec p) possibleWitness)
       (recur possibleWitness p exponent remainder (inc upto))))))

(defn isWitness [possibleWitness p exponent remainder upto]
  (let [possibleWitness (pow possibleWitness remainder p)]
    (cond
     (or (= possibleWitness 1) (= possibleWitness (dec p))) false
     (zero? exponent) true
     :else (checkWitness possibleWitness p exponent remainder upto))))

I've noticed that, in translation, several functions have acquired an extra argument that is used internally to control repetition. I'll just restore decompose:

(defn decompose [n]
  (loop [exponent-of-two 0, n n]
    (if (odd? n)
      {:exponent exponent-of-two :n n}
      (recur (inc exponent-of-two) (quot n 2)))))

The others - isWitness and probablyPrime  - are up to you. If you follow the Python, you don't need to factor out checkWitness.

To do:

I have not seriously tested the above.
Factoring out checkWitness from isWitness can be bettered using
the sequence library.


Finding all prime factors of a number I have a function that is able to return the factors of a number that are prime.

For example: The prime factors of 13195 are 5, 7, 13 and 29.

The function works great, and I was even able to improve it by dividing by just prime numbers instead of all numbers. The problem is it still isn't efficient enough because at 100,000,000 the program takes 10 seconds to finish and the number I need to run through this is 600,851,475,143 which will take more than 6000 times longer to finish.

In the spirit of the challenge, I don't want to be given the answer, but some guidance.

There are also issues with just how large the number is. If I use xrange() it will still give me an overflow error but this isn't a huge problem (I can google answers for that).

def getPrimesM(integer, start=4):
  primes = [2,3]
  for num in range(start, integer):
    if integer % num == 0:
      for i, prime in enumerate(primes):
        if num % prime == 0:
          break
        if i >= len(primes) - 1:
          primes.append(num)

  return primes

Your function will always report that 2 and 3 are prime factors of your number.

Trial division is a fine strategy for this problem, and it can find the factors of 600,851,475,143 quite quickly.  You have to do it smartly, though.  If your main loop is for num in range(start, integer): … or for num in xrange(start, integer): …, and there's no break for that loop, then you're going to be counting up to 600 billion.

Another important detail is that when you find a factor, you should use that information to reduce the integer as much as possible.  If you do it correctly, then you won't need that inner loop for primality testing anymore.

Another useful tip is to test candidates 2, 3, 5, 7, 9, 11, 13, … — in other words, 2 followed by just the odd numbers.  You can write that as

from itertools import chain, count

for num in chain([2], count(3, 2)):
    …


VueJS ReST Client: Vuex Store Methods I'm writing a ReST API Client Tester using VueJS.

I'm trying to have a really clean code, But I feel like it's too much,

Here is my store.js file, which I use it for Vuex and the application state management, all there variables I use to send a request are store here

import Vue from 'vue';
import Vuex from 'vuex';

Vue.use(Vuex);

export default new Vuex.Store({
  state: {
    globals: {
      methods: ['GET', 'POST', 'PUT', 'DELETE'],
    },
    request: {
      method: 'GET',
      url: 'http://example.com',
      body: {},
      headers: {},
    },
  },
  mutations: {

    // Change Methods
    changeMethod(state, method) {
      state.request.method = method;
    },
    changeURL(state, url) {
      state.request.url = url;
    },
    changeBody(state, body) {
      state.request.body = body;
    },
    changeHeaders(state, headers) {
      state.request.headers = headers;
    },

    // Add to Data Methods
    addToHeaders(state, payload) {
      state.request.headers[payload.key] = payload.value;
    },
    addToBody(state, payload) {
      state.request.body[payload.key] = payload.value;
    },
    // Delete from Data Methods
    deleteFromHeaders(state, key) {
      delete state.request.headers[key];
    },
    deleteFromBody(state, key) {
      delete state.request.body[key];
    },

    // Reset Methods
    resetMethod(state) {
      state.request.method = 'GET';
    },
    resetURL(state) {
      state.request.url = '';
    },
    resetBody(state) {
      state.request.body = {};
    },
    resetHeaders(state) {
      state.request.headers = {};
    },

    // Reset request Method
    resetRequest(state) {
      state.request = {
        method: 'GET',
        url: '',
        body: {},
        headers: {},
      };
    },

  },
  actions: {

  },
});

As you can see, I'm sticking to a rule which says, each function should only do one thing and nothing more, I'm not sure if I'm using it right or now, But with this amount of code, It seems a little ridiculous to me...

Let me know what you think? How can I improve this to have more readability and less LoC?

Am I doing it right or not?

Overall, your code seems okay if you have UI elements setting each of these attributes. If you are solely setting these requests in one go with code, having separate mutations is overkill and probably unnecessary.

I think there are some things you can improve though.

Reactivity

The new key-value pairs you add to headers and body are not reactive, because it is not possible to detect addition of new keys in Objects via native methods. I strongly recommend to use Vue.set(..) and Vue.delete(..) when dealing with objects.

Default

You have a reset function that has the same payload as found under the default state. Consider factoring out this part and copying it everytime you need a new state. You need to keep in mind that you require a deep copy to get this to work.

LoC

You can potentially decrease the number of lines of code by creating a generator function that generates your mutations. The thing with a generator function is that it adds an extra layer of abstraction, and will likely make your code less readable than it is now. I don't think factoring out the mutations you have here into a generator function is very helpful either.


Simple slideshow with jQuery I've made a very simple slideshow that works by appending and prepending images (on click and every 3 seconds). This is my first time trying to use classes or objects so any help would be great. I feel this could be a little verbose so I'd love any sort of feedback.

The user can click through the slideshow, but it also works on a timer of every 3 seconds.

 // Scroll through images on the homepage

 function IndexSlideshow() {

    // declare index page html elements

    var contentSlideshow = $('.content-slideshow'),
       slide = $('.slide'),
       body = $('body'),
       self = this;

    // append the first slide, fade in the newly bumped slide

    this.work = function() {
       $('.slide:first').fadeOut(200, function() {
          $(this).appendTo(contentSlideshow);
          $('.slide:first').hide().fadeIn(200);
       });
    }

    // show previous slide on click

    this.showPrev = function() {
       slide.click(function() {
          $(this).prependTo(contentSlideshow);
       });
    }

    // show next slide on click

    this.showNext = function() {
       slide.click(function() {
          self.work();
       });
    }

    // create an auto slide (slides every 3 seconds)

    this.auto = function() {
       var run;

       if (body.hasClass('home')) {
          run = setInterval(function() {
             self.work();
          }, 3000);

          // reset interval if they clicked an image

          slide.click(function() {
             clearInterval(run);
             run = setInterval(function() {
                self.work();
             }, 3000);
          });

       };

    }

 }

 var i = new IndexSlideshow();

 i.showNext();
 i.auto();

I like this code;

There are perhaps a few too many newlines, especially here:

         });

      };

   }

}

work and auto are unfortunate function names, you could do better
slide = $('.slide'), <- I would have called that slides since you are almost bound to select more than 1 slide with this selector
The repeated magical constants 3000 and 200 should be identified, named and centralized


Idiomatic Ruby if/else in a Sinatra route Apologies if this question doesn't fit the format here, seemed more appropriate here than SO due to the more nebulous nature of the question.
Think mini tinyURL clone: POST to service to save the url + hash, then look up the mapping when visiting /anything. It's been a while since I've touched Ruby, and the resulting code snippet is not easy on the eyes:
require 'sinatra'
require 'mongoid'
require 'digest'

class MinifiedUrl
  include Mongoid::Document

  field :code, type: String
  field :original_url, type: String
end

# Redirect to the actual url, like tinyurl.com/abcdefg
get '/:code' do
  link = MinifiedUrl.where(:code => params[:code]).first # .first is falsy if there is no mapping
  if link
    puts "Found minified link"
    redirect link[:original_url], 302
  else
    puts "No minified link"
    redirect '/404', 404
  end
end

# Create a mapping from a POST request like { url: 'https://google.com/ '}
post '/' do
  incoming_url = params[:url]

  link = MinifiedUrl.where(:code => params[:code]).first
  # Reassign link if link is falsy
  link = MinifiedUrl.create(:target_url => incoming_url, :code => Digest::MD5.hexdigest(incoming_url)[0..5]) unless link

  link[:slug]
end

Is there an obvious way to make the code more idiomatic? My concern is multiple calls to MinifiedUrl model / reassigning value of link. I can't put my finger on it, but it doesn't feel ruby-like.

I agree that re-assigning variables is not great. The most 'obvious' way to improve this is to extract a helper method like
def fetch_link
  if (existing_link = MinifiedUrl.where(:code => params[:code]).first)
    existing_link
  else
    MinifiedUrl.create(:target_url => params[:url], :code => generate_code)
end

def generate_code
  Digest::MD5.hexdigest(incoming_url)[0..5]
end

or if you prefer or memoization
def fetch_link
  return existing_link if existing_link.present?

  MinifiedUrl.create(:target_url => params[:url], :code => generate_code)
end

def existing_link
  @existing_link ||= MinifiedUrl.where(:code => params[:code]).first
end

def generate_code
  Digest::MD5.hexdigest(incoming_url)[0..5]
end

Another approach could be to implement a class method on your model which does it for you like
class MinifiedUrl
  def self.find_or_create_by(original_url:, code:)
    if (existing_link = MinifiedUrl.where(:code => code).first)
      existing_link
    else
      MinifiedUrl.create(:target_url => original_url, :code => generate_code)
    end
  end
end

post '/' do
  incoming_url = params[:url]

  link = MinifiedUrl.find_or_create_by(url: incoming_url, code: params[:code])

  link[:slug]
end

## Edit
Adding some more thoughts about error handling here.
URL
There are several things which can go wrong with the URL like invalid or already taken.
The invalid case I would cover with a validation but there is not much what you can validate here to be honest except the format. Mongoid comes with ActiveSupport validations so you could do something like this.
class MinifiedUrl
  validates_format_of :original_url, with: /regex/
end

MinifiedUrl.create(url: 'invalid').valid?
# false

It can be that your URL is already taken, e.g. somebody else already added this URL. What do you want to do in this case? Give an error or allow it? I think it's a valid use case to allow the same URL twice with a different code (you don't want to have the same code as e.g. you want to have a visit counter as well). If you want to allow the same URL twice, your approach with the MD5 hash does not work anymore as it would be the same obviously so in this case you should use something more random.
The hex digest is anyway a problem in case of collisions you would fail at the moment. I think a better approach is to just generate a random code in a loop.
def generate_code
  100.times do
    code = SecureRandom.hex
    break code unless MinifiedUrl.find_by(code: code)
  end
end

I'm also wondering why your POST does accept a code parameter but I don't know your user interface, I just found it a bit confusing.
I wouldn't be too concerned about DB not available and would not put any code handling this in my application to be honest. If you need this safety you should have DB replicas which can handle if a node goes away.
I would also calculate if a 5 digit code is big enough for your use case.
I'm also wondering why you chose to use to Mongoid? I don't think a document store is a good data storage here? I think a key/value store like Redis would be a better alternative here because you basically just do key lookup and you can store everything in a hash. But again, I don't know the requirements.


Enabling discard pending changes on DbContext This code review request is tightly coupled with this SO question, this is the solution I implemented to solve the problem being asked about there.

All my ViewModels get constructor-injected with a Model that itself gets constructor-injected with a DbContext-derived class, and that works well in all cases, except when the View has a command to allow the user to discard pending changes, in which case the DbContext should be disposed and then reinstantiated.

Obviously since the DbContext is created by an IoC container it would be a very bad idea to just dispose and reinstantiate the context, so I came up with a solution involving a factory.

public interface IContextFactory<out TContext> where TContext : DbContext
{
    TContext Create();
}

public interface IDiscardModelChanges
{
    void DiscardChanges();
}

Now IDiscardModelChanges is implemented by the model, so to facilitate this I've created a base class that will ensure I have a context factory at hand:

public abstract class DiscardableModelBase<TContext> : IDiscardModelChanges, IDisposable
    where TContext : DbContext
{
    private readonly IContextFactory<TContext> _factory;

    protected TContext Context { get; private set; }

    protected DiscardableModelBase(IContextFactory<TContext> factory)
    {
        _factory = factory;
        Context = _factory.Create();
    }

    public virtual void DiscardChanges()
    {
        Context.Dispose();
        Context = _factory.Create();
    }

    public void Dispose()
    {
        Context.Dispose();
    }
}

The model that needs to control its DbContext then derives from this class:

public class SomeModel : DiscardableModelBase<SomeContext>, ISomeModel
{
    public SomeModel(IContextFactory<SomeContext> factory)
        : base(factory)
    { }

    /* methods that act upon protected Context property */
}

And then the ViewModel that needs to discard pending changes can do it like this (given a private readonly ISomeModel _model):

var model = _model as IDiscardModelChanges;
if (model != null) model.DiscardChanges();

As far as binding conventions are concerned, I'm keeping the existing "Context" convention, and adding a "ContextFactory" one:

_kernel.Bind(t => t.From(_dataLayerAssembly)
                   .SelectAllClasses()
                   .Where(type => type.Name.EndsWith("ContextFactory"))
                   .BindSingleInterface());

Bottom line, as far as ViewModels are concerned, nothing changes; the Model is still injected as an interface that exposes the available model methods, and the IDiscardModelChanges interface is only needed if the ViewModel needs to use it, and the ViewModel can't assume the interface is implemented by the Model.

Any known issues with this approach, any blatant mistake made?

As stated by previous comments, there is no need to throw away the context.
But since this is a code review, and not a concept review I'll leave that part aside.

All your TContext generics are to be inheriting from DbContext, but nowhere in any class do you use any of DbContext's properties or methods. You are only using the .Dispose method, which comes from the IDisposable interface. I've refactored your code to have this result:

public interface IContextFactory<out TContext> where TContext : IDisposable
{
    TContext Create();
}

public interface IDiscardModelChanges
{
    void DiscardChanges();
}

public abstract class DiscardableModelBase<TContext> : IDiscardModelChanges, IDisposable
where TContext : IDisposable
{
    private readonly IContextFactory<TContext> _factory;

    protected TContext Context { get; private set; }

    protected DiscardableModelBase(IContextFactory<TContext> factory)
    {
        _factory = factory;
        Context = _factory.Create();
    }

    public virtual void DiscardChanges()
    {
        Context.Dispose();
        Context = _factory.Create();
    }

    public void Dispose()
    {
        Context.Dispose();
    }
}

The changes might seem small, but the impact is big. In your example, you are going to pull in Entity Framework everywhere you are using this application, which isn't needed at all.
You are not doing anything entity framework related, you are doing something with disposable objects. This could be a StreamWriter, etc...

PS: I know this post is old. But people should really be aware of the issue with this code.


Leetcode three sum in Javascript I was doing 3sum question on leetcode
Question
Given an array nums of n integers, are there elements a, b, c in nums such that a + b + c = 0? Find all unique triplets in the array which gives the sum of zero.
Note:
The solution set must not contain duplicate triplets.
Example:
Given array nums = [-1, 0, 1, 2, -1, -4]
A solution set is:
[
  [-1, 0, 1],
  [-1, -1, 2]
]

My solution
For this I wrote the following solution but this is giving me Time Limit Exceeded error.

var threeSum = function(nums) {

    // Creating triplet memory so there are any duplicates
    const triplet_memory = {}

    // num occurrence will have all the numbers in the input array and number of time they occured
    const num_occurence = {}

    nums.forEach((element) => {
        if (!num_occurence.hasOwnProperty(element)) {
            num_occurence[element] = 1
        } else {
            num_occurence[element] += 1
        }
    })

    // iterating over input array
    nums.forEach((elParent, indexParent) => {

     // Nested loop so that I try all possible combination
        nums.forEach((elChild, indexChild) => {

            if (indexParent !== indexChild) {

                // decreasing the value of current element from our object
                // created copied_num_mem so that we don't change main object memeory
                const copied_num_mem = {...num_occurence}

                // We are decreasing the elParent and elChild value because for currentSum we are utilizing those value
                // For example if elParent is 1 and elChild = 2, we would be using those value in our currentSum hence we are decreasing their count by 1
                copied_num_mem[elParent] = copied_num_mem[elParent] - 1
                copied_num_mem[elChild] = copied_num_mem[elChild] - 1

                // multiplying by -1 because suppose we have elParent as -1 and elChild as -1, their sum would give us -2 and we would need the reciprocal of -2 i.e 2 to make it positive
                const currentSum = (parseInt(elParent) + parseInt(elChild))*-1

                // Checking if 2 exist in our copied_num_mem and if yes, it's value is greater than 0
                if (copied_num_mem.hasOwnProperty(currentSum.toString()) && copied_num_mem[currentSum.toString()] > 0) {

                  // 2, -1, -1 and -1, 2, -1 all are triplets, we are sorting it so that the order of triplet is always the same and we are going to then store that triplet in our triplet_memory
                  const tripletInt = [currentSum, parseInt(elParent), parseInt(elChild)].sort((a, b) => a -b)
                 const tripletStringified = tripletInt.join('/')
                 triplet_memory[tripletStringified] = true
                }
            }
        })
    })
    const finalErr = []
    Object.keys(triplet_memory).forEach(el => {
        const elements = el.split('/').map((element) => {
            return parseInt(element)
        })
        finalErr.push(elements)
    })
    return finalErr
};

console.dir(threeSum([0,0,0]))
console.dir(threeSum([-1,0,1,2,-1,-4]))

Can someone help me in optimizing the algorithm? I have added comments so it should be easy to understand the code.

Current code
Before discussing the algorithm I want to discuss the current code.
The code currently uses functional approaches - like forEach() methods. This is great for readability but because a function is called for every iteration of each loop, performance can be worse than a regular for loop - e.g.  each function adds to the call stack.
The current code also uses hasOwnProperty. For a plain object the in operator could be used since it doesn't matter if the property would be inherited or not.
The last block is this:

const finalErr = []

Object.keys(triplet_memory).forEach(el => {
    const elements = el.split('/').map((element) => {
        return parseInt(element)
    })
    finalErr.push(elements)
})
return finalErr

It is interesting that there is a .map() call nested inside a .forEach() loop that pushes elements into an array - the latter is the essence of a .map() call. So the .forEach() could be simplified to a .map() call:
return Object.keys(triplet_memory).map(el => {
    return el.split('/').map((element) => {
        return parseInt(element)
    })
})

This way there is no need to manually create finalErr, push elements into it and then return it at the end.
Different Algorithm
There are multiple posts about this problem on code review (and SO as well). This buzzfeed article explains multiple approaches including The hash map solution and the two pointer trick, the latter of those two is a great solution.

Two pointer trick
The ‘two pointer trick’ gives a really nice solution to 3sum that doesn’t require any extra data structures. It runs really quickly and some interviewers ‘expect’ this solution (which might be somewhat unfair, but now that you’re seeing it, it’s to your advantage).
For the two pointer solution, the array must first be sorted, then we can use the sorted structure to cut down the number of comparisons we do. The idea is shown in this picture:

vector<vector<int>> threeSum(vector<int>& nums) {
 vector<vector<int>> output;
 sort(nums.begin(), nums.end());
 for (int i = 0; i < nums.size(); ++i) {
   // Never let i refer to the same value twice to avoid duplicates.
   if (i != 0 && nums[i] == nums[i - 1]) continue;
   int j = i + 1;
   int k = nums.size() - 1;
   while (j < k) {
     if (nums[i] + nums[j] + nums[k] == 0) {
       output.push_back({nums[i], nums[j], nums[k]});
       ++j;
       // Never let j refer to the same value twice (in an output) to avoid duplicates
       while (j < k && nums[j] == nums[j-1]) ++j;
     } else if (nums[i] + nums[j] + nums[k] < 0) {
       ++j;
     } else {
       --k;
     }
   }
 }
 return output;

1


Quadratic equation class in Python OOP Hello everyone
In today's assignment I had to write a function to determine if (and how many) the quadratic function defined by the formula f(x) = ax^2 + bx +c has roots. I had decorators to write.
Here is my solution:
from typing import List, Tuple
from math import sqrt
from datetime import datetime

class DeltaError(Exception):
    '''Error when calculating roots, and delta is lower than 0.'''
    def __init__(self):
        Exception.__init__(self, "Delta must be greater than 0!")

class Quadratic:

    def __init__(self, a: int, b: int, c: int):
        self.a = a
        self.b = b
        self.c = c
        self.x = -1

    @property
    def Roots(self):
        a, b, c = self.a, self.b, self.c
        d = sqrt(b * b - 4 * a * c)
        if d > 0:
            x1 = (-b + d) / (2 * a)
            x2 = (-b - d) / (2 * a)
            return x1, x2
        if d == 0:
            return -b / 2 * a
        if d < 0:
            raise ZeroError

    @property
    def Vietas_formula(self):
        a, b, c = self.a, self.b, self.c
        d = b * b - 4 * a * c
        if d > 0:
            return "x1 + x2 = {x}".format(x = -b / a), "x1 * x2 = {y}".format(y = c / a)

    @property
    def Vertex(self):
        a, b, c = self.a, self.b, self.c
        d = b * b - 4 * a * c
        return "W: ({p}, {q})".format(p = -b / 2 * a, q = -d / 4 * a)

    @property
    def Time_dependent(self):
        if datetime.now().hour + 2 in range(8, 16):
            return "Roots"
        else: return "Break"

    @property
    def x_plus_3(self):
        a, b, c, x = self.a, self.b, self.c, self.x
        x += 3
        return x * (x * a + b) + c

quad = Quadratic(1, 5, 6)
print(quad.Roots, quad.Vietas_formula, quad.Vertex, quad.Time_dependent, quad.x_plus_3)

I'm counting on advice, and a better use of classes, static assignment to a, b, c and delta values, so that through inheritance I can use in subsequent functions without having to declare them from scratch.
Have a nice day!

Some minor stuff -

your member functions and properties should be lower-case by PEP8
you should add a property to get the discriminant, particularly since you use it in multiple places
I don't know what time_dependent and x_plus_3 do, nor why they're here. They seem like specific applications of the quadratic formula for a narrow situation. Given that, they should not be in this class.
The returns from vietas_formula should not be strings; you should return a tuple of floats. Similar for vertex. Currently this is premature stringizing. Your roots already does this correctly.
If you wanted to be thorough, d < 0 should not raise a ZeroError and instead should return complex roots. Python has built-in support for complex numbers.
self.x does not belong as a member on the class.
Have you renamed DeltaError to ZeroError?
Exception.__init__ should be changed to use super()
The error message Delta must be greater than 0! is not strictly true, and should be greater than or equal to zero.
Why must a, b, c be int? You should accept float.

Some major stuff: have you checked this for correctness? I see at least three algebraic errors, including that d = sqrt(b * b - 4 * a * c) is calling sqrt too early to catch failures, and -b / 2 * a has incorrect order of operations. Unit testing for this kind of code is both easy and important; while translating your code I ran into test failures and had to make corrections almost every step of the way.
Suggested
I still don't understand why your code needs to take a break (it's actually pretty funny. Maybe it's unionized?), but so be it:
from cmath import sqrt, isclose
from datetime import datetime, time
from numbers import Complex
from typing import Union, Tuple

RootTypes = Union[
    Tuple[Complex],
    Tuple[Complex, Complex],
]

WORK_HOURS_START = time(8)
WORK_HOURS_END = time(16)

class Quadratic:
    def __init__(self, a: float, b: float, c: float):
        self.a, self.b, self.c = a, b, c

    def y(self, x: Complex) -> Complex:
        a, b, c = self.a, self.b, self.c
        return a*x*x + b*x + c

    def dydx(self, x: Complex) -> Complex:
        a, b = self.a, self.b
        return 2*a*x + b

    @property
    def discriminant(self) -> float:
        a, b, c = self.a, self.b, self.c
        return b*b - 4*a*c

    @property
    def roots(self) -> RootTypes:
        a, b, c, d = self.a, self.b, self.c, self.discriminant
        if d == 0:
            return -b/2/a,

        sqrt_d = sqrt(d)
        return (-b + sqrt_d)/2/a, (-b - sqrt_d)/2/a

    @property
    def vietas_formula(self) -> Tuple[
        float,  # sum
        float,  # product
    ]:
        a, b, c = self.a, self.b, self.c
        return -b/a, c/a

    @property
    def vertex(self) -> Tuple[
        float,  # p
        float,  # q
    ]:
        a, b, c, d = self.a, self.b, self.c, self.discriminant
        return -b/a/2, -d/a/4

    def describe(self) -> str:
        v1, v2 = self.vietas_formula
        return (
            f'Roots: {self.roots}\n'
            f'Vieta constants: x1+x2={v1}, x1*x2={v2}\n'
            f'Vertex: {self.vertex}'
        )

def time_problem() -> None:
    if WORK_HOURS_START <= datetime.now().time() < WORK_HOURS_END:
        a, b, c = 1, 5, 6
        quad = Quadratic(a, b, c)
        print(quad.describe())
    else:
        print("I'm on a break for some reason.")

def abs_close(x: Complex, y: Complex) -> bool:
    return isclose(x, y, abs_tol=1e-12)

def test_two_real() -> None:
    q = Quadratic(1, 5, 6)

    x1, x2 = q.roots
    assert abs_close(0, x1.imag)
    assert abs_close(0, x2.imag)
    assert abs_close(0, q.y(x1))
    assert abs_close(0, q.y(x2))

    v1, v2 = q.vietas_formula
    assert abs_close(v1, x1 + x2)
    assert abs_close(v2, x1 * x2)

    vx, vy = q.vertex
    assert abs_close(0, q.dydx(vx))
    assert abs_close(vy, q.y(vx))

def test_one_real() -> None:
    q = Quadratic(9, -6, 1)

    x, = q.roots
    assert abs_close(0, x.imag)
    assert abs_close(0, q.y(x))

    # In this case Vieta's formula can be interpreted as two identical superimposed roots
    x1, x2 = x, x
    v1, v2 = q.vietas_formula
    assert abs_close(v1, x1 + x2)
    assert abs_close(v2, x1 * x2)

    vx, vy = q.vertex
    assert abs_close(0, q.dydx(vx))
    assert abs_close(0, vy)

def test_two_complex() -> None:
    q = Quadratic(4, -4, 3)

    x1, x2 = q.roots
    assert not abs_close(0, x1.imag)
    assert not abs_close(0, x2.imag)
    assert abs_close(0, q.y(x1))
    assert abs_close(0, q.y(x2))

    v1, v2 = q.vietas_formula
    assert abs_close(v1, x1 + x2)
    assert abs_close(v2, x1 * x2)

    vx, vy = q.vertex
    assert abs_close(0, q.dydx(vx))
    assert abs_close(vy, q.y(vx))

def test() -> None:
    test_two_real()
    test_one_real()
    test_two_complex()

if __name__ == '__main__':
    test()
    time_problem()


Encode strings by subtracting one from each character code I currently have this encoding function which simply subtracts one from each character code:

String.fromCharCode.apply(null, text.split("").map(function(v) {
    return v.charCodeAt() - 1;
}));

E.g. test becomes sdrs.

I know that this function is silly because it isn't a strong encoding algorithm, but that's not my point. The problem is that it is slow and causes a stack overflow for large strings (~130.000 in length).

I tried a regexp but that's even slower:

text.replace(/./g, function(v) {
    return String.fromCharCode(v.charCodeAt() - 1);
});

I tested both on jsPerf.

Currently, I'm executing a function for each character in both functions. How can I make a function that does the same thing as what these functions are doing, but executes faster without stack overflows?

Try looping through it with a simple for loop:

var b = '';
for (var i = 0; i < a.length; i++)
{
   b += String.fromCharCode(a.charCodeAt(i) - 1)
}
return b;


File to String Pattern for uploading to a server I need to upload some images to a server. This is done with a json object. The json object has 2 values, a timestamp and a Base64 encoded string of the image. I am using the jackson library and was wondering if this is a good way to upload the images. I am worried about memory consumption if I happen to upload images asyncronusly in the future. Here is is...

@JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
public class Photo {

    @JsonIgnore
    private String path;
    private String id;
    @JsonProperty("ts")
    private long timestamp;

    public Photo(long timestamp, String path) {
        this.timestamp = timestamp;
        this.path = path;
    }

    public String getData() {
        Bitmap bm = BitmapFactory.decodeFile(path);
        ByteArrayOutputStream baos = new ByteArrayOutputStream();
        bm.compress(Bitmap.CompressFormat.JPEG, 50, baos);
        bm.recycle();
        byte[] byteArray = baos.toByteArray();
        try {
            baos.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
        return Base64.encodeToString(byteArray, Base64.DEFAULT);
    }
}

I'm not familiar with Android at all, so just some generic Java notes:

I'd rename bm to bitmap. A little bit longer variable names are usually easier to read and maintain.
The constructor should validate its input parameters. Does it make sense to call it with null or empty string as path or a less than zero timestamp? If not, check it and throw a NullPointerException or an IllegalStateException. (Effective Java, Second Edition, Item 38: Check parameters for validity)
It is a bad idea to use printStackTrace() in Android exceptions.
I don't know if it differs from general Java or not but closing a ByteArrayOutputStream in Java has no effect. You might be able to omit it.


Print bounding box in the terminal Right now I am finding ways to write better code in javascript and I see many coding styles used by different authors but right now I am thinking to write code which adheres to the object oriented paradigm better suited for this type of language.

Code

// Terminal

'use strict';

class Terminal {
  constructor() {
    this.cols = process.stdout.columns || 80
    this.rows = process.stdout.rows || 60
  }

  width() {
    return this.cols
  }

  height() {
    return this.rows
  }
}

module.exports = Terminal

// Box

'use strict';

const Terminal = require('./terminal')

class Box {
  constructor(opts) {
    let terminal = new Terminal()
    const defaults = {
      w: terminal.width(),
      h: terminal.height(),
      t: '══',
      tr: '╗',
      r: '║',
      br: '╝',
      b: '══',
      bl: '╚',
      l: '║',
      tl: '╔',
      fill: '░░'
    }
    this.settings = Object.assign(defaults, opts)
  }

  toString() {
    const res = [];
    const { t, tr, r, br, b, bl, l, tl } = this.settings
    const { w , h } = this.settings
    // Filling row wise
    for (let i = 0; i < h; i++) {
      for (let j = 0; j < w; j++) {
        if (i === 0 && j === 0) {
          res.push(tl)
        }
        else if (i === 0 && j === w - 1) {
          res.push(tr)
        }
        else if ((i === h - 1) && (j === w - 1)) {
          res.push(br)
        }
        else if ((i === h - 1) && j === 0) {
          res.push(bl)
        }
        else if (this.isFirstRow(i) || this.isLastRow(i)) {
          res.push(t)
        }
        else if (this.isFirstCol(j) || this.isLastCol(j)) {
          res.push(l)
        }
        else {
          const { fill } = this.settings
          res.push(fill)
        }
      }
      res.push('\n')
    }
    return res.join('')
  }

  isFirstRow(row) {
    return row === 0
  }

  isLastRow(row) {
    const { h } = this.settings
    return row === h - 1
  }

  isFirstCol(col) {
    return col === 0
  }

  isLastCol(col) {
    const { w } = this.settings
    return col === w - 1
  }
}

class TopRight {
  constructor() {
    this.symbol = '╗'
  }

  toString() {
    return this.symbol
  }
}

class BottomRight {
  constructor() {
    this.symbol = '╝'
  }

  toString() {
    return this.symbol
  }
}

class BottomLeft {
  constructor() {
    this.symbol = '╚'
  }

  toString() {
    return this.symbol
  }
}

class TopLeft {
  constructor() {
    this.symbol = '╔'
  }

  toString() {
    return this.symbol
  }
}

module.exports = Box

// O/P
/*
> const B = require('./box')
> console.log(new B({w: 8, h: 8, fillSymbol: '||'}).toString())
╔════════════╗
║░░░░░░░░░░░░║
║░░░░░░░░░░░░║
║░░░░░░░░░░░░║
║░░░░░░░░░░░░║
║░░░░░░░░░░░░║
║░░░░░░░░░░░░║
╚════════════╝
*/

I would like to know how to improve the above code to make better use of prototypal inheritance and delegation which is widely advocated in javascript world.

Note

I thought to use classes for printing symbols but I see that they make code more bloated. I left them there hear opinion of others.

This can be order of magnitudes simpler.  You want a function that takes a width, height, and optional fill character, and returns a string (your box).

Try this:

function box(w, h, fill='░░') {
  const top    = '╔' + '══'.repeat(w-2) + '╗\n'
  const row    = '║' + fill.repeat(w-2) + '║\n'
  const bottom = '╚' + '══'.repeat(w-2) + '╝'
  return top + row.repeat(h-2) + bottom
}


Python beginner's tree with unit tests I have implemented a tree in Python. However, due to my lack of experience with Python, I am not able to judge the overall quality of my code.

import unittest
from datetime import *
from gc import *

RET_NO = 0
RET_YES = 1
RET_MAYBE = 0.5 # haha :)

class Tree(object):
    """
    Mutable abstract data type for arbitrary data with O(N) access on elements.
    """
    def __init__(self):
        super(Tree, self).__init__()
        self.root = None

    def insert(self, element):

        if self.root:
            self.root.insert(element)
        else:
            self.root = Node(element)

    def contains(self, element):
        if self.root is None:
            print 'WARN: Contains() called before elements were inserted!!'
            raise Exception()

        return self.root.contains(element) if self.root else RET_NO

    def size(self):
        return self.root.size() if self.root else 0

    def bulk_insert(self, elements=[]):
        while len(elements):
            self.insert(element[0])
            elements = elements[1:]

class Node(object):
    """
    Node class to be used in Tree instances.
    """
    def __init__(self, element):
        super(Node, self).__init__()
        self.element = element
        self.left = None
        self.right = None

    def insert(self, element):
        if self.element == element:
            return
        if element < self.element:
            if self.left:
                self.left.insert(element)
            else:
                self.left = Node(element)
        else:
            if self.right:
                self.right.insert(element)
            else:
                self.right = Node(element)
        raise Exception('Should never reach this line!')

    def contains(self, element):
        if self.element == element:
            return RET_YES
        if element < self.element:
            if self.left:
                return self.left.contains(element)
            else:
                return RET_NO
        else:
            if self.right:
                return self.right.contains(element)
            else:
                return RET_NO
        raise Exception('Should never reach this line!')

    def size(self):
        """
        Return size
        """
        left_size = self.left.size() if self.left else 0
        right_size = self.right.size() if self.right else 0
        return left_size + right_size + 1

class TreeTest(unittest.TestCase):

    def setUp(self):
        # no setup required yet
        pass

    def tearDown(self):
        # leave memory in a clean state
        gc.collect()

    def test_1(self):
        #tree = Tree()
        #self.assertEqual(tree.size(), 0)
        #tree.insert(10)
        #self.assertEqual(tree.size(), 10)
        self.assertTrue(True)

    def test_2(self):
        start = datetime.now()

        elements = []
        for i in range(1000000):
            elements += [i]

        tree = Tree()
        # tree should be empty
        for i in elements:
            self.assertEqual(tree.contains(i), RET_NO)
        # fill tree
        for i in elements:
            tree.insert(i)
        # tree should be full
        for i in elements:
            self.assertEqual(tree.contains(i), RET_YES)

        end = datetime.now()
        elapsed = end - start
        # must not take longer than a minute
        self.assertTrue(elasped.total_seconds() < 60)

    def test_3(self):
        tree = Tree()
        try:
            tree.contains(1)
            # should have thrown an exception -> test failed
            self.assertTrue(False)
        except Exception, ValueError:
            # expected to raise an Exception -> test should pass
            self.assertTrue(True)

Don't do from something import *.

Either import what you need:

from datetime import datetime

Or import the entire module and use qualified names:

import datetime
...
    start = datetime.datetime.now()

Importing everything is dangerous because it can shadow already existing names. One such example:

import datetime
from datetime import *
# `datetime` as a class shadows `datetime` as a module

You might want to do a little reading on the subject.

This whole thing makes no sense:

RET_NO = 0
RET_YES = 1
RET_MAYBE = 0.5 # haha :)

Instead of RET_YES / RET_NO you sould use True and False.

You don't have to call super(...).__init__  if the class only inherits from object.

Why do you print something to stdout and then raise an empty exception?

Instead of

print 'WARN: Contains() called before elements were inserted!!'
raise Exception()

you should do

raise Exception('contains() called on empty tree')

In the method Tree.contains, you already checked if self.root is None, so this

return self.root.contains(element) if self.root else RET_NO

can be simplified to this

return self.root.contains(element)

Instead of contains you can define the __contains__ method. Then you can use the in keyword:

return element in self.root

Although in this particular case it's less readable, so it's probably better to just stick with contains.

Method Tree.bulk_insert is just wrong. There's no point in having an empty list for a default argument. In the for loop you modify the list while iterating over it, and on each iteration you call the len function. Why? Even the C-style

for i in range(len(elements)):
    self.insert(elements[i])

would have been better.

This is the way to do it in Python:

def bulk_insert(self, elements):
    for element in elements:
        self.insert(element)

That way elements doesn't have to be a list, it can be any iterable.

Does your tree have a special requirement that it cannot contain the same element twice? If not, then in Node.insert there should be no

if self.element == element:
    return

Remove unreachable statements. There's no point in having

raise Exception('Should never reach this line!')

in Node.contains, while having it in Node.insert effectively makes the method raise an exception in almost all cases. Actually if you fix the error mentioned in the previous point, it would be in completely all cases.

The comment

"""
Return size
"""

is pointless. The method itself is called size, so it's obvious that it returns size. Also, the name size is quite ambiguous to begin with. Either don't restate code in comments, or actually explain what the method does (eg. how do you compute the size). A more suitable name would be number_of_elements, or maybe something shorter but descriptive like nelems.

The docstring of Node states:

"""
Node class to be used in Tree instances.
"""

I'd say it can possibly be used in other classes than Tree, so don't create a tight coupling between the two, even if it's just in comments.

size can be a property:

@property
def size(self):
    ...

That way you won't have to call it:

left_size = self.left.size if self.left else 0

Even tests should be documented. Add docstrings explaining what do the test_* methods test. Your non-test code also lacks documentation.

The docstring of Tree says "O(N) access on elements". This is not quite true, for two reasons:

your Tree and Node classes don't even allow you to access elements, they only allow you to check whether an element is present in the tree,
\$O(n)\$ is the worst case; average asymptotic complexity is \$O(log(n))\$.

Have you by chance forgotten to implement a remove method? Or you just wanted an insert-only tree?

$$⸻$$


Reading ints line from a file in Python I have to list a integers from a text file separated by newlines into a python list. I ended up with the code above which works (for my case) but certainly is far form optimal.

def readIntegers(pathToFile):

    f = open(pathToFile)
    contents = f.read()
    f.close()

    tmpStr = ""

    integers = []

    for char in contents:

        if char == '\r':
            integers.append(int(tmpStr))
            tmpStr = ""
            continue

        if char == '\n':
            continue

        tmpStr += char

    return integers

Now I have much less code, but I'm not sure for which cases split() works correctly.

def readIntegers(pathToFile):

    with open(pathToFile) as f:
        a = [int(x) for x in f.read().split()]
    return a

No need for split (you can use readlines). But no need for readlines, for that matter:

def read_integers(filename):
    with open(filename) as f:
        return map(int, f)

or, if that’s more to your fancy:

def read_integers(filename):
    with open(filename) as f:
        return [int(x) for x in f]

A file object is simply iterable in Python, and iterates over its lines.

Note that, contrary to what I said earlier, wrapping the file object in a with block is highly recommended. Python doesn’t actually guarantee (although it recommends it) that objects are disposed of automatically at the end of the scope. Worse, it’s not guaranteed that a file object that is collected actually closes the underlying stream.

(Note that I’ve adapted the method name to Python conventions.)


Finding a substring within a string using regular expressions I'm new to Java and is trying to find a substring within a string using regular Expressions. I was just wondering if there are any alternative ways to achieve the same goal, or in a more elegant or efficient manner.

while (true) {

    BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
    System.out.println("\nEnter your regex:");

    Pattern pattern
            = Pattern.compile(br.readLine());

    System.out.println("\nEnter input string to search:");
    Matcher matcher
            = pattern.matcher(br.readLine());

    boolean found = false;
    while (matcher.find()) {
        System.out.format("I found the text"
                + " \"%s\" starting at "
                + "index %d and ending at index %d.%n",
                matcher.group(),
                matcher.start(),
                matcher.end());
        found = true;
    }
    if (!found) {
        System.out.println("No match found.");
    }
}

You code is pretty much good but

BufferedReader br = new BufferedReader(new InputStreamReader(System.in));

may be easily replaced with

Console br = System.console();

Also your while cycle is endless. I would allow to enter some keyword for breaking the loop. E.g. exit

...
System.out.println("\nEnter your regex:");
String input = br.readLine();

if ("exit".equals(input))
    break;
else {
    Pattern pattern = Pattern.compile(input);
    ...
}


Checking if all possible combinations of any two array elements in an array contains 0-9 atleast once I am coding for an online platform. The code is working on some test cases but is getting a Time limit exceeded. What are the ways to make my code more efficient?

  Input

5
129300455
5559948277
012334556
56789
123456879

  Output

5

  Explanation

  Winning Pairs:

  129300455 56789 -->contains all from 0-9 atleast once

  129300455 123456879-->contains all from 0-9 at least once etc

   import java.io.*;
   import java.util.*;
   import java.text.*;
   import java.math.*;
   import java.util.regex.*;

   public class Main {

static int winningLotteryTicket(String[] tickets,int n)
 // tickets = array of all strings

 {
    int count=0;
    boolean b=false;
    // the i and j loop are to create all possible combinations
    for(int i=0;i<n;i++)
{

    for(int j=i+1;j<n;j++)
    {

     if(i!=j)
     { String check=tickets[i]+tickets[j];

     // running loop to check from 0-9 atleast once
     for(int u=0;u<=9;u++)
     {
         String r=u+"";
         b=check.contains(r);
         if(!b)
         {
         break;
         }

     }
     if(b){count++;}
     }
    }
}
    return count;
}

public static void main(String[] args) {
    Scanner in = new Scanner(System.in);
    int n = in.nextInt();
    String[] tickets = new String[n];
    for(int tickets_i = 0; tickets_i < n; tickets_i++){
        tickets[tickets_i] = in.next();
    }
    int result = winningLotteryTicket(tickets,n);
    System.out.println(result);
    in.close();
}
}

static int winningLotteryTicket(String[] tickets,int n)
 // tickets = array of all strings

 {

Please don't put comments in between a function declaration and the function body (which starts with {).

The standard way to write this in Java is

// tickets = array of all strings
static int winningLotteryTicket(String[] tickets, int n) {

This is shorter and easier to read.

Some people prefer to put the { on a separate line.  That's not standard in Java.  But if you do do that, please put the { on the line immediately after the function declaration.  Put comments either before the function declaration or inside the function body.

In Java, unlike C, you do not need to pass the length of the array separately.  Instead of n, you can just use tickets.length.  So

    for (int i = 0; i < tickets.length; i++) {
        for (int j = i + 1; j < tickets.length; j++) {

This also has the advantage of being clearer than n.  I can immediately see that it is the length of the array that is the limitation.  And this avoids bugs where n is passed as something other than the length of the array.

     if(i!=j)

This line is unnecessary.  The way that the inner for loop is declared, j is always greater than i.

         b=check.contains(r);

This is inefficient.  It does a linear time search for each digit.  We can do a single linear time search that covers all the digits.  Consider

public static final int RADIX = 10;

static boolean hasAllDigits(String ticketPair) {
    boolean[] found = new boolean[RADIX];
    for (char digit : ticketPair.toCharArray()) {
        found[digit - '0'] = true;
    }

    for (boolean digitFound : found) {
        if (!digitFound) {
            return false;
        }
    }

    return true;
}

So the original method could be just

static int winningLotteryTicket(String[] tickets) {
    int count = 0;

    // the i and j loop are to create all possible combinations
    for (int i = 0; i < tickets.length; i++) {
        for (int j = i + 1; j < tickets.length; j++) {
            if (hasAllDigits(tickets[i] + tickets[j])) {
                count++;
            }
        }
    }

    return count;
}

If this still isn't fast enough, consider storing the digits found in each string.  E.g. instead of "129300455" store "0123459".  Then you can just use merge logic to check if two strings contain all the digits.


Structure of inheritance between Animal classes and methods I'm trying to do some basic inheritance between the 3 classes and use some simple methods, and I'm looking to learn what should be the correct structure between them and what can be improved.

class Animal:
    def __init__(self, kind, place):
        self.kind = kind
        self.place = place

#This class inherits kind and place arguments from Animal class (which can work for any animal)
#Initiates a Dog object with name, gender and breed parameters.

class Dog(Animal):
    def __init__(self, name, gender, breed):
        self.name = name
        self.gender = gender
        self.breed = breed
        Animal.__init__(self, 'Dog', 'Ground')

    #woof method, just prints an action for the Dog object with his name.
    def woof(self, *args):
        print("%s just did a %s woof" % (self.name, *args))

    #getallinfo method, get's all the parametrs of both classes.
    def getallinfo(self):
        print("%s is a %s %s %s, sitting on the %s" % (self.name, self.gender, self.breed, self.kind, self.place))

#Cat class inherits the paramets of use for a Cat (similar things) like name, gender and breed, which they both share, also the getallinfo method and initiate them.
class Cat(Dog):
    def __init__(self, name, gender, breed):
        Dog.__init__(self, name, gender, breed)
        Animal.__init__(self, 'Cat', 'Ground')

#Speak method, returns a print for a cat "meow" with him name.
    def speak(self, *args):
        print("%s just did a %s meow" % (self.name, *args))

#Here I create 3 objects, 2 dogs and 1 cat with selected arguments.
#And check for some methods on the objects.
Mickey = Dog('Mickey', 'Male', 'Bulldog')
Flora = Dog('Flora','Female','Pug')
Tina = Cat('Tina','Female','Persian')
Tina.getallinfo()
Tina.speak('soft')
Dog.getallinfo(Flora)
Dog.woof(Mickey, 'loud')

Inheritance is typically describe as an "is-a" relationship.  So when you derive a Dog from an Animal, we can say that a Dog is a Animal.  However, when you derive a Cat from a Dog, unless this is some other planet, we can't correctly say that a Cat is a Dog.  Further, we can, without error, invoke Tina.woof('ferocious') to cause "Tina just did a ferocious woof".  Since no Persian cat I've ever seen has been known to "woof" either ferociously or not, this is an alarming and surprising result.

Better would be to derive both Dog and Cat types from Animal.  If you have other animals which don't have names, breeds or genders, you could have some intermediate class such as Pet that would capture the additional detail not already in Animal.  Otherwise, just add those attributes to Animal.

Finally, we can put the speak method in the base Animal class.  One simple illustration is this:

class Animal:
    def __init__(self, kind, place):
        self.kind = kind
        self.place = place

    def speak(self, adverb):
        print("%s just did a %s %s" % (self.name, adverb, self.speechtype))

    #getallinfo method, get's all the parametrs of both classes.
    def getallinfo(self):
        print("%s is a %s %s %s, sitting on the %s" % (self.name, self.gender, self.breed, self.kind, self.place))

#This class inherits kind and place arguments from Animal class (which can work for any animal)
#Initiates a Dog object with name, gender and breed parameters.

class Dog(Animal):
    def __init__(self, name, gender, breed):
        self.name = name
        self.gender = gender
        self.breed = breed
        self.speechtype = "woof";
        Animal.__init__(self, 'Dog', 'Ground')

#Cat class inherits the paramets of use for a Cat (similar things) like name, gender and breed, which they both share, also the getallinfo method and initiate them.
class Cat(Animal):
    def __init__(self, name, gender, breed):
        self.name = name
        self.gender = gender
        self.breed = breed
        self.speechtype = "meow";
        Animal.__init__(self, 'Cat', 'Ground')

#Here I create 3 objects, 2 dogs and 1 cat with selected arguments.
#And check for some methods on the objects.
Mickey = Dog('Mickey', 'Male', 'Bulldog')
Flora = Dog('Flora','Female','Pug')
Tina = Cat('Tina','Female','Persian')
Tina.getallinfo()
Tina.speak('soft')
Flora.getallinfo()
Mickey.speak('loud')

There's more that could be improved, but I hope that helps until others weigh in on your question and give more expansive answers.


Quadratic Expression Calculator 2 After taking some great advice on this question, I'm hoping for some more feedback on my quadratic expression calculator.

This is my code as stands:

public class Quadratic_Equations {

    private final static Scanner sc = new Scanner(System.in);

    public static void main(String[] args) {
        while (true) {
            System.out.println("Please enter a, b and c or enter \"exit\" at any time to exit");
            double a = promptUserUntilValidInput("Please enter a:");
            double b = promptUserUntilValidInput("Please enter b:");
            double c = promptUserUntilValidInput("Please enter c:");
            System.out.println(formatOutput(calculateQuadraticFormula(a, b, c)));
            System.out.println();
        }
    }

    private static String formatOutput(Set<Double> resultsFromEquation) {
        if (resultsFromEquation == null)
            return "These numbers do not compute - they produce an illegal result.";
        else {
            return resultsFromEquation.toString();
        }
    }

    private static double promptUserUntilValidInput(String prompt){
        if (prompt.equals("exit")) {
            sc.close();
            System.exit(0);
        }
        boolean valid = false;
        Double value = new Double(0.0);
        while(!valid){
            System.out.println(prompt);
            value = tryParse(sc.nextLine());
            if (value != null)
                valid = true;
        }
        return value;
    }

    private static Double tryParse(String text){
        try{
            return Double.parseDouble(text);
        }
        catch (NumberFormatException e){
            return null;
        }
    }

    public static Set<Double> calculateQuadraticFormula(double a, double b, double c) {
        Set<Double> results = new HashSet<Double>();
        double temp = (b * b) - (4 * a * c);
        if (temp < 0)
            return null;
        if( temp == 0 ) {
            results.add(-b / 2 * a);}
        else if( temp > 0) {
            results.add( (-b + Math.sqrt(temp) ) / (2 * a) );
            results.add( (-b - Math.sqrt(temp) ) / (2 * a) );
        }
        return results;
    }
}

Before I go on, there are a few things I should point out.

I'm aware this could be better if the quadratic stuff was in a class of its own - however, for the purposes of this simple exercise I'm happy skipping that.

My main issues are:

static Scanner. Some people on previous question suggested making this a private variable, but then it needs to get passed to everything. Is this worth it?
Does it look okay to close the scanner (and exit the program) where I do?
Do other methods look single-responsibility-ish enough?

Bugs

calculateQuadraticFormula() calculates wrong results for double roots.  Watch your operator associativity!

if( temp == 0 ) {
    results.add(-b / 2 * a);}

The "exit" feature doesn't work.  The prompt would never be "exit", would it?

private static double promptUserUntilValidInput(String prompt){
    if (prompt.equals("exit")) {
        sc.close();
        System.exit(0);
    }

Design issues

Class names should be nouns, and the name should reflect their purpose.  I recommend QuadraticSolver as the class name.
The calculateQuadraticFormula() function could then be simply named solve().
The class should not hold a Scanner as a member variable, as that would violate the Single Responsibility Principle.  It can be a local variable in main() that gets passed to the prompting function.  (The scanner should be created using a try-with-resources block.)
Returning null from the solver is annoying for the caller to handle.  I suggest returning an empty set if there are no real roots.
I suggest returning two copies of the result if it is a double root, but that is a matter for debate.

Style issues

You assign the expression (b * b) - (4 * a * c) to a variable named temp.  Why not use a descriptive name discriminant?
The promptUserUntilValidInput() function could be better if you removed its tryParse() helper, I think.  (Furthermore, promptUser… is redundant — who else are you going to prompt?)

Also, since there is a System.exit() hidden inside, it needs to be documented.  (Actually, @rolfl's suggestion to throw an exception is better.)

/**
 * Prompts the user to enter a number, and retries until the input is a
 * valid double.  Calls System.exit(0) if the input is "exit", or if EOF
 * is encountered.
 */
private static double promptUntilValidInput(String prompt, Scanner sc) {
    do {
        try {
            System.out.print(prompt);
            String input = sc.nextLine();
            if ("exit".equals(input)) {
                System.exit(0);
            }
            return Double.parseDouble(input);
        } catch (NoSuchElementException eof) {
            System.out.println();
            System.exit(0);
        } catch (NumberFormatException retryOnBadInput) {
        }
    } while (true);
}


Project Euler problem 92 in Scala, square digit chains Here is a summary of the problem:
"A number chain is created by continuously adding the square of the digits in a number to form a new number until it has been seen before... How many starting numbers below ten million will arrive at 89?" The site also gives an example at https://projecteuler.net/problem=92.

I have a solution which arrives at the correct answer but takes ~24 seconds on my computer to complete. I used an array to record as true the index of each value which resolves to 89 through the chain described above. I used an array so that updating and checking values would take constant time. I believed that recording all numbers that resolved to 89 would save time so that the checkAll function could avoid redundant calls of chain for these values.

However, I tested this assumption and wrote a version in which chain would simply recurse until resolving to true or false without recording the chain, so that checkAll would check every single value from 1 until 10 million but perhaps save time from not recording values. Despite the change, this version took almost the exact same amount of time, although at least it's more concise.

For the above reason I now suspect that the chain of method calls used to produce the next variable in the chain function is the main bottleneck, but I am not sure whether this is possible to implement in a faster way.

object euler92 {
  def main(args: Array[String]): Unit = {
    val t0 = System.nanoTime()

    val blank = new Array[Boolean](10000000)
    def chain(number: Int, links: List[Int] = List()): Unit = {
      val next = number.toString.toList.map(_.toString).map(_.toInt).map(x => x * x).sum
      if (next == 89) for (i <- number :: links) blank(i) = true
       else if (next == 1) ()
      else {
        chain(next, number :: links)
      }
    }
    def checkAll(num: Int, counter: Int = 0): Unit = {
      if (num == 10000000) println("Answer: " + counter)
      else if (blank(num) == true) checkAll(num + 1, counter + 1)
      else {
        chain(num)
        if (blank(num) == true) checkAll(num + 1, counter + 1)
        else checkAll(num + 1, counter)
      }
    }
    checkAll(1)

    val t1 = System.nanoTime()
    println("Elapsed time: " + (t1 - t0) + " ns")
  }
}

Here is the implementation without an array:

def chain(number: Int, links: List[Int] = List()): Boolean = {
      val next = number.toString.toList.map(_.toString).map(_.toInt).map(x => x * x).sum
      if (next == 89) true
      else if (next == 1) false
      else {
        chain(next, number :: links)
      }
    }

def checkAll(num: Int, counter: Int = 0): Unit = {
  if (num == 10000000) println("Answer: " + counter)
  else {
    val ans = chain(num)
    if (ans) checkAll(num + 1, counter + 1)
    else checkAll(num + 1, counter)
  }
}
checkAll(1)

I am hoping for insight on how to improve the performance of this solution.

Let's start off with your code to convert to the sum of digits.

number.toString.toList.map(_.toString).map(_.toInt).map(x => x * x).sum

First of all, a String can already be treated as List[Char], so that toList is unnecessary. Also, instead of converting from Char to String to Int, you can just call asDigit. And while we're at it, let's stuff that in a function, so that we know what's going on when we call that line. After all that, the line looks like:

def sumSquareDigits(number: Int): Int = number.toString.map(_.asDigit).map(x => x*x).sum

Now, you had the right idea in creating a hash. However, as an array of Booleans it's hard to tell what's been set and what hasn't. For now, let's start with a HashMap, since it's the most obvious construct for a cache. If it were too slow you could worry more about optimization, but at least on my computer it's sufficient to solve it. This has the added advantage of not needing to cache values that aren't repeated. Instead of needing to fill in 10,000,000 integers (I don't have that kind of spare RAM), it only ends up needing to cache 495 values. Additionally, since we're using the cache inductively, we should prepopulate the cache with our base case:

val cache = scala.collection.mutable.HashMap[Int,Int](1->1,89->89)

Now, we want to see whether a value converges to 1 or 89. We can do this by recursively calling the hash on the sumSquareDigits defined above until we get an answer, placing the results of any new stuff into the cache. We'll use a helper function just for simplicity:

def addValueToCache(number: Int) =
{
  val converged = getConvergedValue(number)
  cache.put(number,converged)
  converged
}
def getConvergedValue(number: Int) = cache.getOrElse(sumSquareDigit(x), addValueToCache(sumSquareDigit(x))

And now we just need to check which values converge to 89,

def checkAll(max: Int): Int=
  (1 until max).map(getConvergedValue).filter(_ == 89).size

then call it from main!

def main(args: Array[String]): Unit =
{
  println("Answer: "+checkAll(10000000))
}

Notice how using if statements for flow control is entirely unnecessary here. Functional programming style tends to do flow control with monadic operations (filter, map, flatMap, flatten, etc...) or pattern matching. If you're trying to stick to a functional style, when you see a lot of nested if statements you should be wondering if there's a better way to do it.


Refactor ConvertorToString class Help me refactor this class that helps to represent an object state as String:

public class ConvertorToString {
    private static final String SEPARATOR_BETWEEN_FIELD_NAME_AND_VALUE = "=";
    private static final String SEPARATOR_BETWEEN_FIELDS = ", ";
    private static final String OPEN_FIELDS = "( ";
    private static final String CLOSE_FIELDS = " )";

    private final String className;
    private String[] fieldNames;
    private Object[] fieldValues;

    public static ConvertorToString buildFor(Object someObject) {
        return new ConvertorToString(someObject.getClass().getName());
    }

    private ConvertorToString(String className) {
        this.className = className;
    }

    public ConvertorToString withFieldNames(String... fieldNames) {
        this.fieldNames = fieldNames;
        return this;
    }

    public ConvertorToString withFieldValues(Object... fields) {
        this.fieldValues = fields;
        return this;
    }

    private class Field {
        private final int index;

        public Field(int index) {
            this.index = index;
        }

        public String toString() {
            checkIndex(index);
            return fieldNames[index] + SEPARATOR_BETWEEN_FIELD_NAME_AND_VALUE + fieldValues[index];
        }

        private void checkIndex(int index) {
            if (index < 0 || index >= countOfFields()) {
                throw new IllegalArgumentException("Index should be inside a range: " +
                        "[0," + countOfFields() + "];  " + "but index was: " + index);
            }
        }
    }

    public String toString() {
        checkState();
        return className
               + OPEN_FIELDS +
                StringsCombiner.combine(getFields(), SEPARATOR_BETWEEN_FIELDS)
               + CLOSE_FIELDS;
    }

    private void checkState() {
        if (fieldNames == null || fieldValues == null || fieldNames.length != fieldValues.length) {
            throw new IllegalStateException("count of fieldNames should be equal to count of fieldValues");
        }
    }

    private List<Field> getFields() {
        List<Field> fields = new ArrayList<Field>(countOfFields());
        int countOfFields = countOfFields();
        for (int i = 0; i < countOfFields; ++i) {
            fields.add(new Field(i));
        }
        return fields;
    }

    private int countOfFields() {
        return fieldNames.length;
    }
}

Using:

class SomeClass {
    int id;
    String title;

    @Override
    public String toString() {
        return ConvertorToString.buildFor(this)
            .withFieldNames("id", "title")
            .withFieldValues(id, title)
            .toString();
    }
}

I like the idea of this utility class, but there are a few points I would like to mention.

Let's start with naming. I would rename the ConvertorToString class, probably, to one of the following ToStringUtil, ToStringHelper, ObjectToString or ObjectToStringConvertor. The rule of thumb is to put the -or/-er nouns in the end of the class name. Personally, I like the first two. Then come the constant fields which could be shorter. For instance, they could be named as FIELD_NAME_AND_VALUE_SEPARATOR instead of SEPARATOR_BETWEEN_FIELD_NAME_AND_VALUE and FIELDS_SEPARATOR instead of SEPARATOR_BETWEEN_FIELDS. That is, the 'BETWEEN' part is redundant because it's already obvious that a separator separates the one from the other. Two other fields would be better named as FIELDS_START and FIELDS_END, which is more intuitive.

Another thing is that you have two separate methods for adding the field names and values and then you check that there is a corresponding value for each field name in the ConvertorToString.checkState() method. You also check for indexes and arrays' length in the ConvertorToString.Field.checkIndex() method. All these checks are error-prone, but they could be eliminated by enforcing the one-to-one correspondence rule of the field names and values in a single method for adding those fields and their values:

ToStringUtil.addField(String name, Object value);
ToStringUtil.addFields(Map<String, Object> fields);

This way you control the state of your object and have no need in the ConvertorToString.checkState() and ConvertorToString.Field.checkIndex() methods anymore. Moreover, internally I would store the field names and values in one data structure, rather than in two separate arrays. The single structure could be a Map<String, Object> or a List<Field>, the latter one is preferable if you have a Field class. This brings us to the ConvertorToString.Field class.

I think, it would be more practical to encapsulate the field names and values within the ConvertorToString.Field class, which will yield a better OOP-style design. Also, this change will allow you to transform the ConvertorToString.Field class from an inner class to a static nested class, because you don't need the reference to the outer class anymore. After you encapsulate the field name and its value in the ConvertorToString.Field class, the FIELD_NAME_AND_VALUE_SEPARATOR field would naturally belong to the ConvertorToString.Field class. And because the class name is Field, the FIELD_NAME_AND_VALUE_SEPARATOR field could further be renamed to NAME_AND_VALUE_SEPARATOR.

One other opinion on the ConvertorToString constants would be that you don't actually need them because you don't use them anywhere else besides the toString() methods. So, you could just use the string literals instead:

public class ToStringConvertor {
    private final String className;
    private final List<Field> fields;
    ...
    @Override
    public String toString() {
        return String.format(
            "%s(%s)",
            className,
            StringsCombiner.combine(fields, ", ")
        );
    }

    private static class Field {
        private final String name;
        private final Object value;
        ...
        @Override
        public String toString() {
            return String.format("%s=%s", name, value);
        }
    }
}


Angular callback scope.apply Usually the socket.io's on method is like this:

socket.on('/msg', function(data){});

But in Angular that callback has to be wrapped inside its $rootScope.$apply() method otherwise anything that happens in the callback (like changing $scope properties) doesn't update in Angular.

Here's a simple way to do that:

function socketFactory($rootScope) {
    return {
        emit: socket.emit.bind(socket),
        on: function(msg, callback) {
            socket.on(msg, function() {
                var args = arguments;
                $rootScope.$apply(function(){
                    callback.apply(socket, arguments)
                });
            });
        },
    };
};

I was able to shorten this a little bit:

on: function(msg, callback) {
    socket.on(msg, function() {
        $rootScope.$apply(callback.apply(socket, arguments));
    });
},

I'm not sure if it'll go any further but I tried:

on: function(msg, callback) {
    socket.on(msg, $rootScope.$apply.apply(null, callback));
},

which threw an error:

  Uncaught TypeError: Cannot read property 'apply' of undefined socket.io.js:1194

I'm guessing $apply.apply doesn't work.

.bind should've been used here instead of .apply

on: function(msg, callback) {
    socket.on(msg, $rootScope.$apply.bind($rootScope, callback.bind.apply(socket, arguments)));
},

But seeing how complicated the shortened "one-liner" becomes, expanded form was probably better:

on: function(msg, callback) {
    socket.on(msg, function(data){
        $rootScope.$apply(function(){
            callback(data);
        });
    });
},


Table rendering helper I wrote this little helper to render a table, however, working with nested content_tag is messy.

  def semantic_table(table_headers)
     content_tag(:table, class: 'ui basic striped table') do
       (content_tag :thead do
         content_tag(:tr) do
           table_headers.each { |h| concat content_tag(:th, h) }
         end
       end).+ content_tag(:tbody) { yield }
     end
  end

For example

# users/index.html.slim
= semantic_table ['Username', 'Email', 'Registration date'] do
  = render @users

That's the only way I could make it work, without overusing concat. This code smells off, how do I refactor it?

I would also like to extract _user.html.slim partial's functionality to the helper, though I don't know how

 # _user.html.slim
 tr
   td = link_to user.name, user
   td = mail_to user.email
   td = time_tag user.created_at

The answer, which may not be what you were hoping to hear, is that for generating more than a couple lines of HTML, partials are a much better fit than content_tag et al. Put your table code in a partial in e.g. views/shared/_semantic_table.slim.html:

table class="ui basic striped table"
  thead
    tr
      - headers.each do |header|
        th = header
  tbody
    = yield

...and then make your helper render it with the given arguments:

def semantic_table(headers, &block)
  render partial: "shared/semantic_table",
    locals: { headers: headers }, &block
end

Helpers are great for encapsulating logic that doesn't belong in a view/partial or in the controller, or adds a lot of noise: formatting data, choosing/combining several attributes, that sort of thing. But when you just want to spit out a bunch of boring HTML, you can't really beat a simple partial.

Edit

Re: your comment, that's interesting. I'm guessing Rails is using instance_eval somewhere, so your block with render @users is being evaluated in some different context where @users is nil. This is surprising, but I don't have a computer in front of me to dig further.

There may be a workaround that I'm not thinking of, but barring that, the simplest solution is to drop the helper entirely and just use the partial directly in your view:

= render partial: "shared/semantic_table", locals: { headers: headers } do
  = render @users

I'm not 100% sure that'll work, since didn't anticipate the other issue, but I think it will.

Another solution is to get rid of the block and pass in the objects directly:

def semantic_table(headers, objects)
  render partial: "shared/semantic_table",
    locals: { headers: headers, objects: objects }
end

...and then in the partial call render objects instead of yield.

Of course, this is more constraining than a block, but it may suffice for your use case.


Occurrence of a word in a sentence I have written Java code that finds the frequency of the occurrence of a word in a sentence.  It is working fine.  The only thing I want to do is to optimize this code.

String str = "I am a Boy I am a";
        String[] splitStr = str.split(" ");
        int count = 0;
        List<String> list = new ArrayList<>();
        for(String s:splitStr){
            if(!list.contains(s)){
                list.add(s);
            }
        }
        for(int i=0;i<list.size();i++){
            for(int j=0;j<splitStr.length;j++){
                if(list.get(i).equals(splitStr[j])){
                    count++;
                }
            }
            System.out.println("Occurrence of " + list.get(i) + " is " + count + " times.");
            count=0;
        }

You can simplify your code to a great extent by using a Map<String, Integer> instead of List<String>. The map would store the mapping from each word to it's count in the array.

You can modify your code like so:

String str = "I am a Boy I am a";
String[] splitStr = str.split(" ");

Map<String, Integer> wordCount = new HashMap<>();
for (String word: splitStr) {
    if (wordCount.containsKey(word)) {
        // Map already contains the word key. Just increment it's count by 1
        wordCount.put(word, wordCount.get(word) + 1);
    } else {
        // Map doesn't have mapping for word. Add one with count = 1
        wordCount.put(word, 1);
    }
}

And then simply iterate over the map, and print the key: value pair:

for (Entry<String, Integer> entry: map.entrySet()) {
    System.out.println("Count of : " + entry.getKey() +
                       " in sentence = " + entry.getValue());
}


Interleave two lists of strings I've got a fairly simple task here, but I think it's worth trying to perfect the implementation, because I can probably learn a thing or two about using .NET's enumerables correctly in the process.

I have two "lists" of strings, let's call them values and separators. The former is one element longer than the latter, and I want to interleave them into a single string (so I want to riffle the separators in between the values). Example:

var values = new []{ "abc", "def", "ghi" };
var separators = new []{ "123", "456" };

// expected result: "abc123def456ghi"

I need this in a few places, and in my actual code these might be Lists or other enumerables, so I figured I'd add an extension method to IEnumerable<string>, which I call on the values and pass the separators as an argument. Here's what I've got:

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

namespace Retina.Extensions
{
    public static class EnumerableExtension
    {
        public static string Riffle(this IEnumerable<string> values, IEnumerable<string> separators)
        {
            if (values.Count() != separators.Count() + 1)
                throw new Exception("Riffled enumerable needs to be one shorter than this enumerable.");

            var builder = new StringBuilder();

            var zipped = values.Zip(separators, (val, sep) => new { val, sep });

            foreach (var pair in zipped)
            {
                builder.Append(pair.val);
                builder.Append(pair.sep);
            }

            builder.Append(values.Last());

            return builder.ToString();
        }
    }
}

As I understand it, IEnumerable is generally a lazy list, so both Count() and Last() potentially incur an additional iteration through the entire list, which seems unnecessary. Also, the intermediate zipped enumerable seems awkward for something as simple as alternatingly iterating through two lists.

Is there a more idiomatic way to implement this and make better use of .NET's collections/enumerables? Is IEnumerable even the correct choice here?

You're right about enumerating both parameters multiple times. Each call to Count or Last will execute the enumerator. This might of course become very inefficient, especially if your collections have a lot of items.

So what could you do instead? Well, one option would be to replace IEnumerable with the next closest type which is the ICollection. This would guarantee that both collections are already materialized. This however is rarely a good solution so it's preferable to keep it lazy.

But for this, simple LINQ is not enough. You need to work with enumerators directly and implement your extension in such a way that it uses the GetEnumerator method. With it you have the complete control over the enumeration process. With a little bit of logic you can keep you parameters lazy and still be able to validate that the are more values then separators.

You do this by first getting the first value. Then the while will try to get a pair of separators and values to append them to the string. If at some point it could get a separator but not a value, it means that there is at least the same number of separators as values and at this point you can throw an exception. The other case is when there are not enough separators. To detect this you just need to try to get another value after the while. If it works then it means there were not enough separators.

public static string Join(this IEnumerable<string> values, IEnumerable<string> separators)
{
    var result = new StringBuilder();

    using (var value = values.GetEnumerator())
    using (var separator = separators.GetEnumerator())
    {
        if (value.MoveNext())
        {
            result.Append(value.Current);

            while (separator.MoveNext())
            {
                if (value.MoveNext())
                {
                    result
                        .Append(separator.Current)
                        .Append(value.Current);
                }
                else
                {
                    throw new ArgumentException("There are too many separators!");
                }
            }

            if (value.MoveNext())
            {
                throw new ArgumentException("There are not enough separators!");
            }
        }

        return result.ToString();
    }
}

I would also call this extension Join because this is what it does.