質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう！

質問する

ただいまの
回答率

88.10%

サーブレットでのエラーチェックについて

受付中

回答 1

投稿 ・編集

  • 評価
  • クリップ 0
  • VIEW 9,169
e-power-ma

score 40

やりたいこと
userId：null
pass:nullのとき、エラーを表示したいのですが、サーブレットにif文がいっぱいできてしまいました。
もっとスッキリ書きたいのですが、アドバイスいただけないでしょうか。
また、ログインIDを入力してください。のログインIDの部分を変数にしてと言われていて、詰まっています。
フォワード先のjspも併せて記載します。

一般的にエラーがメッセージを表示させるのに、配列をしようすると先輩に聞いたのですが、
できそうにないので、インスタンスに文字列を格納してフォワードで送っています。 

import java.io.IOException;

import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;

import model.AccountLogic;
import model.Login;

@WebServlet("/LoginServlet")
public class LoginServlet extends HttpServlet {
    private static final long serialVersionUID = 1L;

    protected void doGet(HttpServletRequest request,
            HttpServletResponse response)
            throws ServletException, IOException {

        //フォワード
        RequestDispatcher dispatcher = request.getRequestDispatcher("/WEB-INF/Login.jsp");
        dispatcher.forward(request, response);
    }

    protected void doPost(HttpServletRequest request,
            HttpServletResponse response)
            throws ServletException, IOException {


        //リクエストパラメータの取得
        request.setCharacterEncoding("UTF-8");
        String userId = request.getParameter("userId");
        String pass = request.getParameter("pass");


        //ログイン処理の実行
        Login rs = new Login();
        rs.setUserId(userId);
        rs.setPass(pass);
        AccountLogic bo = new AccountLogic();
        boolean result = bo.execute(rs);

        //ログインの成否によって処理を分岐
        if(userId == null){
            Human human = new Human("ログインIDを入力してください。");
            //リクエストスコープにインスタンスを保存
              request.setAttribute("human", human);

              //リクエストスコープからインスタンスを取得
              Human h = (Human) request.getAttribute("human");
              RequestDispatcher dispatcher = request.getRequestDispatcher("/WEB-INF/Login.jsp");
              dispatcher.forward(request, response);

        }else if(pass == null){
                Human human = new Human("パスワードを入力してください。");
                //リクエストスコープにインスタンスを保存
                  request.setAttribute("human", human);

                  //リクエストスコープからインスタンスを取得
                  Human h = (Human) request.getAttribute("human");
                  RequestDispatcher dispatcher = request.getRequestDispatcher("/WEB-INF/Login.jsp");
                  dispatcher.forward(request, response);
            }



        if (result) { //ログイン成功時

            //セッションスコープにユーザーIDを保存
            HttpSession session = request.getSession();
            session.setAttribute("userId", userId);

            //フォワード
            RequestDispatcher dispatcher = request.getRequestDispatcher("/ListScreen");
            dispatcher.forward(request, response);
        } else {
            //ログイン失敗時
            //
                  //インスタンス生成
                  Human human = new Human("ログインに失敗しました。ログインIDまたはパスワードが違います。");

                  //リクエストスコープにインスタンスを保存
                  request.setAttribute("human", human);

                  //リクエストスコープからインスタンスを取得
                  Human h = (Human) request.getAttribute("human");

                  RequestDispatcher dispatcher = request.getRequestDispatcher("/WEB-INF/Login.jsp");
                  dispatcher.forward(request, response);

                }
        }
    }

コード
<!DOCTYPE HTML>
<html lang="ja">
<head>
<%@ page language="java" contentType="text/html; charset=Windows-31J"
    pageEncoding="Windows-31J" %>

<title>Sample</title>
<%
     //ここでリクエストスコープを受け取る
     //getAttributeメソッド
     //追加した属性を取り出す
     Human human = (Human) request.getAttribute("human");
%>
<%= human.getName() %>

</head>
<body>
    <form id="form" method="post" action="">
        <span id="input_error"></span>

        <p>
            <input type="text" name="user" id="user" />
        </p>

        <p>
            <input type="password" name="password" id="password" />
        </p>

        <p>
            <button id="btn" type="button">送信</button>
        </p>

    </form>
    <script type="text/javascript">
        function validate() {
            var user = document.getElementById('user');
            var password = document.getElementById('password');
            var message = document.getElementById('input_error');
            var err = [];
            if (user.value.toString() === '') {
                err.push('ログインIDが入力されていません。');
            }
            if (password.value.toString() === '') {
                err.push('パスワードが入力されていません。');
            }
            if (err.length > 0) {
                message.innerHTML = err.join('<br>');
                return false;
            }
            return true;
        }

        var btn = document.getElementById('btn');
        btn.addEventListener('click', function(e) {
            if (validate()) {
                document.getElementById('form').submit();
            }
        });
    </script>
</body>
</html>
<script type="text/javascript">
    function formcheck() {
        //必須チェック
        if (document.Form1.userId.value == "") {
            //エラーを打ち出す
            document.getElementById('input_error').innerHTML += "ログインIDが入力されていません。";
        }
        if (document.Form1.pass.value == "") {
            document.getElementById('input_error').innerHTML += "パスワードが入力されていません。";
        }
    }
</script>

コード
package servlet;

import java.io.Serializable;

public class Human implements Serializable {
    private String name;

    public Human(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
}
package model;

public class Login {
private String userId;
private String pass;

public String getUserId() {
    return userId;
}
public void setUserId(String userId) {
    this.userId = userId;
}
public String getPass() {
    return pass;
}
public void setPass(String pass) {
    this.pass = pass;
}

}
package model;

import dao.AccountDAO;

public class AccountLogic {
    public boolean execute(Login login) {
        AccountDAO dao = new AccountDAO();
        Account account = dao.findByLogin(login);

        return account != null;
    }
}

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

more_vert12 件の質問への追記・修正依頼

質問への追記・修正、ベストアンサー選択の依頼

  • e-power-ma

    e-power-ma

    2018/07/07 19:01

    よろしくお願いします。

    キャンセル

  • e-power-ma

    e-power-ma

    2018/07/07 19:06

    ほんとは、配列にエラーメッセージをaddして、ループさせて表示させたいのです。

    キャンセル

  • m.ts10806

    m.ts10806

    2018/07/07 23:30

    時間がかかりましたが回答しました。　　一応・・・配列もインスタンス化したものもどちらも「オブジェクト」です。「オブジェクトとは何か」というところも調べておいてください。用語とか用法部分になりますが、概念的には知っておく必要はあると思います。

    キャンセル

回答 1 件

+3

まず最初に。
JavaScriptでバリデーションしているコードは一通り除いておいてください。
同じような処理を書いているコードが２箇所にありますし、
いきなり全部盛ってしまうのはコードがグチャグチャになる原因となります。

JavaBeansについては下記の記事がわかりやすいと思います。

既に参考にされたようですが、今のやり方と「javaBeansに文字が格納できるんだ」という理解だけだとオブジェクト指向のメリットが全くいかせていないことになります。
記事もあるように「JavaBeans＝銀行」とあります。
銀行に口座を開設して、適宜、情報を保存したり取り出したりして使うものですね。

今のコードを見ますと、
同名の銀行を開設しまくっています。

本来は「データ」を確認して利用するためなので、「文字を保存する」となると意味がありません。
「文字を保存する」だけでいいなら、わざわざBeansにする意味がありません。

あと「エラーメッセージ」というのでしたらそういう意味を込めた名前をつけないと使うときに分かりづらくなります。

今の書き方だと1つずつしか送っていないのでBeans使うまでもなく、文字だけ送る作りと使い方が変わりません。

一気にすべてやろうとするのではなく、何段階かにわけて書いていってください。

注記：動作確認をすべて行っているわけではありません。構文エラーなどは自身で調整してください。
いきなり実装を考えるのではなく、ロジックを考えるというところを優先させてください。

 第１段階　直接エラーメッセージを送る OR 変数に入れて送る

if(userId == null){
    Human human = new Human();
    request.setAttribute("error", "ログインIDを入力してください。");
    RequestDispatcher dispatcher = request.getRequestDispatcher("/WEB-INF/Login.jsp");
    dispatcher.forward(request, response);
}else if(pass == null){
    request.setAttribute("error", "パスワードを入力してください。");
    RequestDispatcher dispatcher = request.getRequestDispatcher("/WEB-INF/Login.jsp");
    dispatcher.forward(request, response);
}

※doPostからだけ送るとNull参照となるのでdoGetでもerrorを空文字""でsetAttribute()しておく 

<%
String error = (String) request.getAttribute("error");
if(!"".equals(error)){
%>
<%=error %>
<%
}
%>

もしくはエラーメッセージをためておく変数を持っておくとか。
こうすると、幾つか同じ記述があるのでまとめることができると気づくかもしれません。 

String error = "";
if(userId == null){
  error = "ログインIDを入力してください。";
}else if(pass == null){
  error =  "パスワードを入力してください。";
}
if(!"".equals(error)){
  request.setAttribute("error ", error);
  RequestDispatcher dispatcher = request.getRequestDispatcher("/WEB-INF/Login.jsp");
  dispatcher.forward(request, response);
}


※JSPは同じ

 第２段階　配列にエラーメッセージをセット

ArrayList<String> error = new ArrayList<>();

if(userId == null){
  error.add("ログインIDを入力してください。");
}
if(pass == null){
  error.add("パスワードを入力してください。");
}
if(error.size() > 0){
  //setしてdispatcher　同じ
}


jsp
※doPostからだけ送るとNull参照となるのでdoGetでもerrorを空配列でsetAttribute()しておく 

<%
ArrayList<String> error = (ArrayList<String>) request.getAttribute("error");
if(error.size() > 0){
  for(int i = 0; i < error.size() ; i++){
%>
   <%=error[i] %><br />
<%
 }
}
%>

 第３段階　チェック用の関数を作る

「を入力してください」のところが共通なので、そこだけ可変にしてチェックするような関数を作る。

例： 

    public static String checkEmpty(String value,String name){
        if(value == null || "".equals(value) ){
            return name + "を入力してください";            
        }
        return "";
    }


※同じクラス内の別メソッドよりの呼び出し 

String error = checkEmpty("","名前");

/*実行結果*/
// 名前を入力してください


メッセージが取れたら第１段階と同じです。

 第４段階　バリデート用のメソッドを作る

第３段階では入力項目の数だけ呼び出し側で書かなければいけませんし、
配列に入れようと思った場合、結果が空文字""かどうかを確認する必要があります。
例： 

ArrayList<String> error = new ArrayList<>();

String errorcheck = "";

errorcheck = checkEmpty(userId,"ログインID");
if(!"".equals(errorcheck)){
    error.add(errorcheck);
}
errorcheck = checkEmpty(pass,"ログインID");
if(!"".equals(errorcheck)){
    error.add(errorcheck);
}

これでは本筋の目的が認証処理であるdoPost()がゴチャゴチャしてきます。
ということで、別途validateメソッドを作ってrequestをそのまま投げて、そちらですべてチェックする方法をとります。

例 

public static ArrayList<String> validate(HttpServletRequest request){
    ArrayList<String> error = new ArrayList<>();
    String userId = request.getParameter("userId");
    String pass = request.getParameter("pass");

    String errorcheck = "";

    errorcheck = checkEmpty(userId,"ログインID");
    if(!"".equals(errorcheck)){
        error.add(errorcheck);
    }
    errorcheck = checkEmpty(pass,"ログインID");
    if(!"".equals(errorcheck)){
        error.add(errorcheck);
    }
    return error;
}

呼び出し元（doPost） 

ArrayList<String> error = validate(request);

if(error.size() > 0){
  //setしてdispatcher　同じ
}

※ArrayList<String> errorをLoginServletのプロパティとして宣言しておくと
validate()の冒頭で宣言する必要も呼び出し元で受け取る必要もありません。
その辺りは今後で。

 第５段階 Beansについて

原則なのですが、持っておくのは「データ」なので
「エラーメッセージ」のようにその後も保持しておく必要のない
１度だけの使い捨て情報については保持しておく必要はないものと考えます。

ただ保持しておくべきデータについては、基本として全てのフィールドに対してget,setを設置する必要があります。
その部分は参考にされた記事の通りですね。
privateで変数を定義→セッター・ゲッター設置→任意のタイミングで利用　です。
※Eclipseなど開発統合環境のエディタでは自動で作ってくれる機能があります

あくまで「銀行」です。
１フィールド１口座と思ってください。
インスタンス化をすると銀行自体が開設されるので、
基本は何度もnewする必要はありません。
同じ銀行を何度も開設するのは変ですよね？
１度開設したらその銀行内のそれぞれの口座に情報を入れていってください。

なお、現在はコンストラクタでいきなり何か入れていっていますが、
コンストラクタはあまりそういう使い方はしないと思います。
「コンストラクタ　使い方」で調べて文献を読み漁ってください。

今回用意されたHumanに対しては下記のような使い方になるかと思います。

あくまで例なのでそのまま書かないこと。足りない部分や仕様と違う部分は自身で埋めること 

public class Human implements Serializable {
    private String name;
    private Integer UserId;
    public Human() {
    }

//ゲッターセッター部分は省略
}

※それぞれのフィールドに対してゲッターセッターが用意されていること 

if (result) { //ログイン成功時
    Human human = new Human();
    human.setName(/*取得してきたユーザ名*/);
    human.setUserId(/*取得してきたユーザID*/);

    session.setAttribute("human", human); 
    RequestDispatcher dispatcher = request.getRequestDispatcher("/ListScreen");
    dispatcher.forward(request, response);
}


JSP側はgetを(Human)でキャストして受け取る。

認証データなので名前はAuthUserとかの方が良いですね。
そもそもHumanクラスが何のために用意されたのかわかりませんが。

強いてなら、validate()メソッドをすべてHumanクラスに移動して、
下記のようにするのはありですが、

呼び出し元（doPost） 

Human human = new Human();
ArrayList<String> error = human.validate(request);

if(error.size() > 0){
  //setしてdispatcher　同じ
}


※この場合はvalidate()メソッドのstaticは外しておいてください。staticではなくなるはずなので。

Humanだけに存在しているのは勿体ないですね。

あとはset～～のときに型が決まっているので、そこでチェックするのもありかもしれません。

ただ、validate()メソッド自体は他の処理で汎用的に使えたほうがいいですよね。
Humanクラスに置くのは進めません。別途Vaidationクラスを作って共通化を検討してください。

共通化ができれば、あとは処理を追加していくだけで良いですし、共通部分と個別部分をわけて考えられます。
個別の情報のみ渡して、あとは分岐して、とか、設定ファイルに持たせるとか、考えられるようになってくるでしょう。

ひとまずBeansのことを考えるのはきちんと第４段階まで出来てからにした方が良さそうです。

 他気づき

  • バリデーション前にログイン処理をしている
    入力値に不正な値が入ってきてたら不具合が起きます
  • LoginクラスとAccountLogicクラスは何？分ける必要があるのだろうか。使い道が不明
  • 謎の記述→ Human h = (Human) request.getAttribute("human"); 
    このgetAttribute()はまったく意味がない。
  • jsp側　不要記述が多い。もう使ってない記述もあるのでは？
    最初に書いたように、JavaScriptでのバリデーションはひとまず一度全て忘れてサーブレットのみで作りこんでいってください。
    どうしても欲しいなら後から合体させれば良いです。

未使用記述・不要記述は実装上での邪魔にしかなりません。
１行１行意味を持たせて、必要な記述のみしていってください。
コピペコードが山積していくとわけがわからなくなって余計混乱します。

ざっと書きましたが、
全部一気にやるのではなく、１段階ずつ動作を確実にし、ステップアップしていってください。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2018/07/08 16:26

    ん、第２は配列のことしか触れていませんが…「も」とは？

    キャンセル

  • 2018/07/08 18:44

    「も」はおかしいですね。

    キャンセル

  • 2018/07/10 13:39

    色々取り組まれているようですが、あちらこちらに手を出していてはどれも中途半端になりかねませんし、
    ストレスもたまりやすくなります。
    やるときは1つに集中、やらないときは完全休養　とメリハリをつけるようにしてください。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう！

  • ただいまの回答率 88.10%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる
質問する

関連した質問

同じタグがついた質問を見る

SNSアカウントでログイン

teratailアカウントでログイン

思考するエンジニアのためのQ&Aサイト「teratail」について詳しく知る